[WebAssembly][GlobalISel] Implement COPY#197256
Conversation
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
COPYCOPY
|
@llvm/pr-subscribers-llvm-globalisel @llvm/pr-subscribers-backend-webassembly Author: Demetrius Kanios (QuantumSegfault) ChangesAdds instruction select handling and tests for Full diff: https://github.com/llvm/llvm-project/pull/197256.diff 2 Files Affected:
diff --git a/llvm/lib/Target/WebAssembly/GISel/WebAssemblyInstructionSelector.cpp b/llvm/lib/Target/WebAssembly/GISel/WebAssemblyInstructionSelector.cpp
index 5c27dd9b5b201..74cd8018e61bb 100644
--- a/llvm/lib/Target/WebAssembly/GISel/WebAssemblyInstructionSelector.cpp
+++ b/llvm/lib/Target/WebAssembly/GISel/WebAssemblyInstructionSelector.cpp
@@ -23,6 +23,7 @@
#include "llvm/CodeGen/MachineOperand.h"
#include "llvm/CodeGen/TargetLowering.h"
#include "llvm/IR/IntrinsicsWebAssembly.h"
+#include "llvm/Support/ErrorHandling.h"
#define DEBUG_TYPE "wasm-isel"
@@ -46,6 +47,7 @@ class WebAssemblyInstructionSelector : public InstructionSelector {
private:
bool selectImpl(MachineInstr &I, CodeGenCoverage &CoverageInfo) const;
+ bool selectCopy(MachineInstr &I, MachineRegisterInfo &MRI) const;
const WebAssemblyTargetMachine &TM;
// const WebAssemblySubtarget &STI;
@@ -83,13 +85,82 @@ WebAssemblyInstructionSelector::WebAssemblyInstructionSelector(
{
}
+bool WebAssemblyInstructionSelector::selectCopy(
+ MachineInstr &I, MachineRegisterInfo &MRI) const {
+ Register DstReg = I.getOperand(0).getReg();
+ Register SrcReg = I.getOperand(1).getReg();
+
+ const TargetRegisterClass *DstRC;
+ if (DstReg.isPhysical()) {
+ switch (DstReg.id()) {
+ case WebAssembly::SP32:
+ DstRC = &WebAssembly::I32RegClass;
+ break;
+ case WebAssembly::SP64:
+ DstRC = &WebAssembly::I64RegClass;
+ break;
+ default:
+ reportFatalInternalError("Copy to physical register other than SP32 or SP64?");
+ }
+ } else {
+ DstRC = MRI.getRegClassOrNull(DstReg);
+ }
+
+ if (!DstRC) {
+ const RegisterBank *DstBank = MRI.getRegBankOrNull(DstReg);
+ if (!DstBank)
+ reportFatalInternalError("Selecting copy with dst reg with no bank?");
+
+ DstRC =
+ TRI.getConstrainedRegClassForOperand(I.getOperand(0), MRI);
+ MRI.setRegClass(DstReg, DstRC);
+ }
+
+
+ const TargetRegisterClass *SrcRC;
+ if (SrcReg.isPhysical()) {
+ switch (SrcReg.id()) {
+ case WebAssembly::SP32:
+ SrcRC = &WebAssembly::I32RegClass;
+ break;
+ case WebAssembly::SP64:
+ SrcRC = &WebAssembly::I64RegClass;
+ break;
+ default:
+ llvm_unreachable("Copy from physical register other than SP32 or SP64?");
+ }
+ } else {
+ SrcRC = MRI.getRegClassOrNull(SrcReg);
+ }
+
+ if (!SrcRC) {
+ const RegisterBank *SrcBank = MRI.getRegBankOrNull(SrcReg);
+ if (!SrcBank)
+ reportFatalInternalError("Selecting copy with src reg with no bank?");
+
+ SrcRC =
+ TRI.getConstrainedRegClassForOperand(I.getOperand(0), MRI);
+ MRI.setRegClass(SrcReg, SrcRC);
+ }
+
+
+ if (DstRC != SrcRC) {
+ reportFatalInternalError("Copy between vregs with mismatching classes?");
+ }
+
+ return true;
+}
+
bool WebAssemblyInstructionSelector::select(MachineInstr &I) {
MachineBasicBlock &MBB = *I.getParent();
MachineFunction &MF = *MBB.getParent();
MachineRegisterInfo &MRI = MF.getRegInfo();
- if (!I.isPreISelOpcode())
+ if (!I.isPreISelOpcode()) {
+ if (I.isCopy())
+ return selectCopy(I, MRI);
return true;
+ }
if (selectImpl(I, *CoverageInfo))
return true;
diff --git a/llvm/test/CodeGen/WebAssembly/GlobalISel/instructions/copy.mir b/llvm/test/CodeGen/WebAssembly/GlobalISel/instructions/copy.mir
new file mode 100644
index 0000000000000..f0d612cc2c11c
--- /dev/null
+++ b/llvm/test/CodeGen/WebAssembly/GlobalISel/instructions/copy.mir
@@ -0,0 +1,146 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 6
+# RUN: llc -mtriple=wasm32-unknown-unknown -run-pass=regbankselect,instruction-select %s -o - | FileCheck %s
+
+---
+name: copy_i32
+legalized: true
+tracksRegLiveness: true
+body: |
+ bb.1.entry:
+ liveins: $arguments
+
+ ; CHECK-LABEL: name: copy_i32
+ ; CHECK: liveins: $arguments
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: [[ARGUMENT_i32_:%[0-9]+]]:i32 = ARGUMENT_i32 0, implicit $arguments
+ ; CHECK-NEXT: RETURN [[ARGUMENT_i32_]], implicit-def $arguments
+ %0:i32(i32) = ARGUMENT_i32 0, implicit $arguments
+ %1:_(i32) = COPY %0(i32)
+ RETURN %1(i32), implicit-def $arguments
+...
+
+---
+name: copy_i64
+legalized: true
+tracksRegLiveness: true
+body: |
+ bb.1.entry:
+ liveins: $arguments
+
+ ; CHECK-LABEL: name: copy_i64
+ ; CHECK: liveins: $arguments
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: [[ARGUMENT_i64_:%[0-9]+]]:i64 = ARGUMENT_i64 0, implicit $arguments
+ ; CHECK-NEXT: RETURN [[ARGUMENT_i64_]], implicit-def $arguments
+ %0:i64(i64) = ARGUMENT_i64 0, implicit $arguments
+ %1:_(i64) = COPY %0(i64)
+ RETURN %1(i64), implicit-def $arguments
+...
+
+---
+name: copy_f32
+legalized: true
+tracksRegLiveness: true
+body: |
+ bb.1.entry:
+ liveins: $arguments
+
+ ; CHECK-LABEL: name: copy_f32
+ ; CHECK: liveins: $arguments
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: [[ARGUMENT_f32_:%[0-9]+]]:f32 = ARGUMENT_f32 0, implicit $arguments
+ ; CHECK-NEXT: RETURN [[ARGUMENT_f32_]], implicit-def $arguments
+ %0:f32(f32) = ARGUMENT_f32 0, implicit $arguments
+ %1:_(f32) = COPY %0(f32)
+ RETURN %1(f32), implicit-def $arguments
+...
+
+---
+name: copy_f64
+legalized: true
+tracksRegLiveness: true
+body: |
+ bb.1.entry:
+ liveins: $arguments
+
+ ; CHECK-LABEL: name: copy_f64
+ ; CHECK: liveins: $arguments
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: [[ARGUMENT_f64_:%[0-9]+]]:f64 = ARGUMENT_f64 0, implicit $arguments
+ ; CHECK-NEXT: RETURN [[ARGUMENT_f64_]], implicit-def $arguments
+ %0:f64(f64) = ARGUMENT_f64 0, implicit $arguments
+ %1:_(f64) = COPY %0(f64)
+ RETURN %1(f64), implicit-def $arguments
+...
+
+---
+name: copy_to_sp32
+legalized: true
+tracksRegLiveness: true
+body: |
+ bb.1.entry:
+ liveins: $arguments
+
+ ; CHECK-LABEL: name: copy_to_sp32
+ ; CHECK: liveins: $arguments
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: [[ARGUMENT_i32_:%[0-9]+]]:i32 = ARGUMENT_i32 0, implicit $arguments
+ ; CHECK-NEXT: $sp32 = COPY [[ARGUMENT_i32_]]
+ ; CHECK-NEXT: RETURN implicit-def $arguments
+ %0:i32(i32) = ARGUMENT_i32 0, implicit $arguments
+ $sp32 = COPY %0(i32)
+ RETURN implicit-def $arguments
+...
+
+---
+name: copy_to_sp64
+legalized: true
+tracksRegLiveness: true
+body: |
+ bb.1.entry:
+ liveins: $arguments
+
+ ; CHECK-LABEL: name: copy_to_sp64
+ ; CHECK: liveins: $arguments
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: [[ARGUMENT_i64_:%[0-9]+]]:i64 = ARGUMENT_i64 0, implicit $arguments
+ ; CHECK-NEXT: $sp64 = COPY [[ARGUMENT_i64_]]
+ ; CHECK-NEXT: RETURN implicit-def $arguments
+ %0:i64(i64) = ARGUMENT_i64 0, implicit $arguments
+ $sp64 = COPY %0(i64)
+ RETURN implicit-def $arguments
+...
+
+---
+name: copy_from_sp32
+legalized: true
+tracksRegLiveness: true
+body: |
+ bb.1.entry:
+ liveins: $arguments
+
+ ; CHECK-LABEL: name: copy_from_sp32
+ ; CHECK: liveins: $arguments
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: [[COPY:%[0-9]+]]:i32 = COPY $sp32
+ ; CHECK-NEXT: RETURN [[COPY]], implicit-def $arguments
+ %0:_(i32) = COPY $sp32
+ RETURN %0(i32), implicit-def $arguments
+...
+
+---
+name: copy_from_sp64
+legalized: true
+tracksRegLiveness: true
+body: |
+ bb.1.entry:
+ liveins: $arguments
+
+ ; CHECK-LABEL: name: copy_from_sp64
+ ; CHECK: liveins: $arguments
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: [[COPY:%[0-9]+]]:i64 = COPY $sp64
+ ; CHECK-NEXT: RETURN [[COPY]], implicit-def $arguments
+ %0:_(i64) = COPY $sp64
+ RETURN %0(i64), implicit-def $arguments
+...
|
f140963 to
987e7cc
Compare
987e7cc to
03fdf93
Compare
| if (DstRC != SrcRC) | ||
| return false; |
There was a problem hiding this comment.
This check doesn't make sense; COPY between classes is one of the primary reasons to use a copy?
There was a problem hiding this comment.
Maybe it used to be, and maybe it still is for other targets. But now that LLT i32 and f32 are distinct, there is no overlap. Each legal LLT is assigned it's own unique class, so what used to be s32 => s32 copy across two regbanks and regclasses is now simply a separate G_BITCAST. I'm leaving that condition there.
Can always be revised later if we end up running into it down the line.
There was a problem hiding this comment.
This may have been a already selected COPY with mismatched classes , this will fail I the nop-select case
There was a problem hiding this comment.
Nop-select?
Like G_SELECT between the same value?
How could they have mismatching classes? When would legalization ever create a copy between two different regclasses, when there is a 1 : 1 mapping between regclasses and valid LLT. COPY is explicitly enforced to only be copy between the same type. And regclass corresponds to one type (LLT::integer(32) => i32regbank => i32 (class)) And I'm doing everything I can to avoid s32, which WOULD cause this problem.
There was a problem hiding this comment.
No, like you already had a selected machine copy between register classes. i.e. %0:some_rc = COPY %1:other_rc
There was a problem hiding this comment.
Okay...maybe. Do you have any particular examples of why/when this would happen? When would the pipeline skip to a fully selected COPY between potentially mismatched classes?
There was a problem hiding this comment.
There isn't a firm boundary between gMIR and MIR. Fully selected target instructions are allowed to be introduced at any point. COPY is just a special case because it's reused between generic vregs and normal vregs. You can test this by just writing a ordinary machine copy in a mir test
There was a problem hiding this comment.
Okay, I've extended the COPY to support transforming it into an appropriate reinterpret. Basically, we allow COPY to act as a G_BITCAST. However this still does not support COPY across different sized reg classes, nor copies to/from "physical" registers with mismatching classes.
Is this enough?
f0f0c64 to
aa6a80b
Compare
aa6a80b to
bfbb0fd
Compare

Adds instruction select handling and tests for
COPY. WhileCOPYgets dissolved in the end, these changes are required to keep the selector from crashing when trying to assign a regclass to its operands.Split from #157161