Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RISCV][GISEL] legalize, regbankselect, and instruction-select G_PTRMASK #73062

Merged
merged 5 commits into from
Nov 30, 2023

Conversation

michaelmaitland
Copy link
Contributor

@michaelmaitland michaelmaitland commented Nov 22, 2023

This is done in instruction-select instead of in legalization for the sake of alias analysis.

@llvmbot
Copy link
Collaborator

llvmbot commented Nov 22, 2023

@llvm/pr-subscribers-backend-risc-v

@llvm/pr-subscribers-llvm-globalisel

Author: Michael Maitland (michaelmaitland)

Changes

G_PTRMASK is custom legalized by using G_PTRTOINT on the pointer, using a G_AND to calculate the mask, and converted back to pointer using G_PTRTOINT.


Full diff: https://github.com/llvm/llvm-project/pull/73062.diff

4 Files Affected:

  • (modified) llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp (+23)
  • (modified) llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.h (+3)
  • (added) llvm/test/CodeGen/RISCV/GlobalISel/legalizer/rv32/legalize-ptrmask.mir (+24)
  • (added) llvm/test/CodeGen/RISCV/GlobalISel/legalizer/rv64/legalize-ptrmask.mir (+24)
diff --git a/llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp b/llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp
index 9eb5812e024b915..1c1c4ee6a9aed3a 100644
--- a/llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp
+++ b/llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp
@@ -147,6 +147,8 @@ RISCVLegalizerInfo::RISCVLegalizerInfo(const RISCVSubtarget &ST) {
 
   getActionDefinitionsBuilder(G_PTR_ADD).legalFor({{p0, sXLen}});
 
+  getActionDefinitionsBuilder(G_PTRMASK).customFor({{p0, sXLen}});
+
   getActionDefinitionsBuilder(G_PTRTOINT)
       .legalFor({{sXLen, p0}})
       .clampScalar(0, sXLen, sXLen);
@@ -288,6 +290,25 @@ bool RISCVLegalizerInfo::legalizeShlAshrLshr(
   return true;
 }
 
+bool RISCVLegalizerInfo::legalizePtrMask(MachineInstr &MI,
+                                         MachineIRBuilder &MIRBuilder,
+                                         GISelChangeObserver &Observer) const {
+  assert(MI.getOpcode() == TargetOpcode::G_PTRMASK);
+
+  MachineRegisterInfo &MRI = *MIRBuilder.getMRI();
+  Register Tmp1 =
+      MRI.createGenericVirtualRegister(MRI.getType(MI.getOperand(2).getReg()));
+  Register Tmp2 =
+      MRI.createGenericVirtualRegister(MRI.getType(MI.getOperand(2).getReg()));
+  MIRBuilder.buildPtrToInt(Tmp1, MI.getOperand(1).getReg());
+  MIRBuilder.buildAnd(Tmp2, Tmp1, MI.getOperand(2).getReg());
+  MIRBuilder.buildIntToPtr(MI.getOperand(0).getReg(), Tmp2);
+
+  Observer.erasingInstr(MI);
+  MI.eraseFromParent();
+  return true;
+}
+
 bool RISCVLegalizerInfo::legalizeCustom(LegalizerHelper &Helper,
                                         MachineInstr &MI) const {
   MachineIRBuilder &MIRBuilder = Helper.MIRBuilder;
@@ -309,6 +330,8 @@ bool RISCVLegalizerInfo::legalizeCustom(LegalizerHelper &Helper,
     return Helper.lower(MI, 0, /* Unused hint type */ LLT()) ==
            LegalizerHelper::Legalized;
   }
+  case TargetOpcode::G_PTRMASK:
+    return legalizePtrMask(MI, MIRBuilder, Observer);
   }
 
   llvm_unreachable("expected switch to return");
diff --git a/llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.h b/llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.h
index f39d3a130d85063..ff2be0622023795 100644
--- a/llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.h
+++ b/llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.h
@@ -31,6 +31,9 @@ class RISCVLegalizerInfo : public LegalizerInfo {
 private:
   bool legalizeShlAshrLshr(MachineInstr &MI, MachineIRBuilder &MIRBuilder,
                            GISelChangeObserver &Observer) const;
+
+  bool legalizePtrMask(MachineInstr &MI, MachineIRBuilder &MIRBuilder,
+                       GISelChangeObserver &Observer) const;
 };
 } // end namespace llvm
 #endif
diff --git a/llvm/test/CodeGen/RISCV/GlobalISel/legalizer/rv32/legalize-ptrmask.mir b/llvm/test/CodeGen/RISCV/GlobalISel/legalizer/rv32/legalize-ptrmask.mir
new file mode 100644
index 000000000000000..7de567a765c0c00
--- /dev/null
+++ b/llvm/test/CodeGen/RISCV/GlobalISel/legalizer/rv32/legalize-ptrmask.mir
@@ -0,0 +1,24 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc -mtriple=riscv32 -run-pass=legalizer -verify-machineinstrs %s -o - | FileCheck %s
+
+---
+name:            ptrmask_p0_s32
+body:             |
+  bb.0:
+    liveins: $x10, $x11
+    ; CHECK-LABEL: name: p0_s32
+    ; CHECK: liveins: $x10, $x11
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: [[COPY:%[0-9]+]]:_(p0) = COPY $x10
+    ; CHECK-NEXT: [[COPY1:%[0-9]+]]:_(s32) = COPY $x11
+    ; CHECK-NEXT: [[PTRTOINT:%[0-9]+]]:_(s32) = G_PTRTOINT [[COPY]](p0)
+    ; CHECK-NEXT: [[AND:%[0-9]+]]:_(s32) = G_AND [[PTRTOINT]], [[COPY1]]
+    ; CHECK-NEXT: [[INTTOPTR:%[0-9]+]]:_(p0) = G_INTTOPTR [[AND]](s32)
+    ; CHECK-NEXT: $x10 = COPY [[INTTOPTR]](p0)
+    ; CHECK-NEXT: PseudoRET implicit $x10
+    %0:_(p0) = COPY $x10
+    %1:_(s32) = COPY $x11
+    %2:_(p0) = G_PTRMASK %0(p0), %1(s32)
+    $x10 = COPY %2(p0)
+    PseudoRET implicit $x10
+...
diff --git a/llvm/test/CodeGen/RISCV/GlobalISel/legalizer/rv64/legalize-ptrmask.mir b/llvm/test/CodeGen/RISCV/GlobalISel/legalizer/rv64/legalize-ptrmask.mir
new file mode 100644
index 000000000000000..b7223c1e2ca2ea6
--- /dev/null
+++ b/llvm/test/CodeGen/RISCV/GlobalISel/legalizer/rv64/legalize-ptrmask.mir
@@ -0,0 +1,24 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc -mtriple=riscv64 -run-pass=legalizer -verify-machineinstrs %s -o - | FileCheck %s
+
+---
+name:            ptrmask_p0_s64
+body:             |
+  bb.0:
+    liveins: $x10, $x11
+    ; CHECK-LABEL: name: ptrmask_p0_s64
+    ; CHECK: liveins: $x10, $x11
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: [[COPY:%[0-9]+]]:_(p0) = COPY $x10
+    ; CHECK-NEXT: [[COPY1:%[0-9]+]]:_(s64) = COPY $x11
+    ; CHECK-NEXT: [[PTRTOINT:%[0-9]+]]:_(s64) = G_PTRTOINT [[COPY]](p0)
+    ; CHECK-NEXT: [[AND:%[0-9]+]]:_(s64) = G_AND [[PTRTOINT]], [[COPY1]]
+    ; CHECK-NEXT: [[INTTOPTR:%[0-9]+]]:_(p0) = G_INTTOPTR [[AND]](s64)
+    ; CHECK-NEXT: $x10 = COPY [[INTTOPTR]](p0)
+    ; CHECK-NEXT: PseudoRET implicit $x10
+    %0:_(p0) = COPY $x10
+    %1:_(s64) = COPY $x11
+    %2:_(p0) = G_PTRMASK %0(p0), %1(s64)
+    $x10 = COPY %2(p0)
+    PseudoRET implicit $x10
+...

G_PTRMASK is custom legalized by using G_PTRTOINT on the pointer, using a
G_AND to calculate the mask, and converted back to pointer using G_PTRTOINT.
@topperc
Copy link
Collaborator

topperc commented Nov 25, 2023

Is this based off another target? I see AArch64 handles this in instruction selection.

@michaelmaitland
Copy link
Contributor Author

Is this based off another target? I see AArch64 handles this in instruction selection.

I didn’t see why this opcode couldn’t be handled generally for all targets. That sounded like it fit well where I put it. No reason why it needs to be in a target specific instruction selector. Maybe other targets can eventually reuse the generic logic added here.

@topperc
Copy link
Collaborator

topperc commented Nov 25, 2023

Is this based off another target? I see AArch64 handles this in instruction selection.

I didn’t see why this opcode couldn’t be handled generally for all targets. That sounded like it fit well where I put it. No reason why it needs to be in a target specific instruction selector. Maybe other targets can eventually reuse the generic logic added here.

If all targets should lower it in the legalizer then why does it exist as an opcode at all?

@tschuett
Copy link
Member

G_PTRMASK is for alias analysis. It should give better results if it exists longer in the pipeline and is only selected at the end. If every target goes through the p->int and int->p early, you will get worse results. Furthermore, GPUs with funny address spaces may do it completely different.

@michaelmaitland
Copy link
Contributor Author

G_PTRMASK is for alias analysis. It should give better results if it exists longer in the pipeline and is only selected at the end. If every target goes through the p->int and int->p early, you will get worse results. Furthermore, GPUs with funny address spaces may do it completely different.

I will move this to instruction selection. Are any targets doing alias analysis between legalization and instruction-selection?

@tschuett
Copy link
Member

The LoadStoreOpt pass uses AliasAnalysis.

@michaelmaitland michaelmaitland changed the title [RISCV][GISEL] Legalize G_PTRMASK [RISCV][GISEL] legalize, regbankselect, and instruction-select G_PTRMASK Nov 27, 2023
Copy link
Collaborator

@topperc topperc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@michaelmaitland michaelmaitland merged commit a6f7278 into llvm:main Nov 30, 2023
3 checks passed
michaelmaitland added a commit that referenced this pull request Dec 8, 2023
G_VAARG can be expanded similiar to SelectionDAG::expandVAArg through
LegalizerHelper::lower. This patch implements the lowering through this
style of expansion.

The expansion gets the head of the va_list by loading the pointer to
va_list. Then, the head of the list is adjusted depending on argument
alignment information. This gives a pointer to the element to be read
out of the va_list. Next, the head of the va_list is bumped to the next
element in the list. The new head of the list is stored back to the
original pointer to the head of the va_list so that subsequent G_VAARG
instructions get the next element in the list. Lastly, the element is
loaded from the alignment adjusted pointer constructed earlier.

This change is stacked on #73062.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants