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 G_VAARG through expansion. #73065

Merged
merged 10 commits into from
Dec 8, 2023

Conversation

michaelmaitland
Copy link
Contributor

@michaelmaitland michaelmaitland commented Nov 22, 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.

@llvmbot
Copy link
Collaborator

llvmbot commented Nov 22, 2023

@llvm/pr-subscribers-llvm-globalisel

Author: Michael Maitland (michaelmaitland)

Changes

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.


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

5 Files Affected:

  • (modified) llvm/include/llvm/CodeGen/GlobalISel/LegalizerHelper.h (+1)
  • (modified) llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp (+67)
  • (modified) llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp (+5)
  • (added) llvm/test/CodeGen/RISCV/GlobalISel/legalizer/rv32/legalize-vaarg.mir (+40)
  • (added) llvm/test/CodeGen/RISCV/GlobalISel/legalizer/rv64/legalize-vaarg.mir (+40)
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/LegalizerHelper.h b/llvm/include/llvm/CodeGen/GlobalISel/LegalizerHelper.h
index 86d3cb2bedb95b6..350c91ad6fa4f61 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/LegalizerHelper.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/LegalizerHelper.h
@@ -430,6 +430,7 @@ class LegalizerHelper {
   LegalizeResult lowerVectorReduction(MachineInstr &MI);
   LegalizeResult lowerMemcpyInline(MachineInstr &MI);
   LegalizeResult lowerMemCpyFamily(MachineInstr &MI, unsigned MaxLen = 0);
+  LegalizeResult lowerVAArg(MachineInstr &MI);
 };
 
 /// Helper function that creates a libcall to the given \p Name using the given
diff --git a/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp b/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
index dd5577d47f97764..310b71dca37bf3c 100644
--- a/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
@@ -3780,6 +3780,8 @@ LegalizerHelper::lower(MachineInstr &MI, unsigned TypeIdx, LLT LowerHintTy) {
     return lowerTRUNC(MI);
   GISEL_VECREDUCE_CASES_NONSEQ
     return lowerVectorReduction(MI);
+  case G_VAARG:
+    return lowerVAArg(MI);
   }
 }
 
@@ -7865,6 +7867,71 @@ LegalizerHelper::lowerVectorReduction(MachineInstr &MI) {
   return UnableToLegalize;
 }
 
+static Type *getTypeForLLT(LLT Ty, LLVMContext &C);
+
+LegalizerHelper::LegalizeResult LegalizerHelper::lowerVAArg(MachineInstr &MI) {
+  Observer.changingInstr(MI);
+  MachineFunction &MF = *MI.getMF();
+  const DataLayout &DL = MIRBuilder.getDataLayout();
+  LLVMContext &Ctx = MF.getFunction().getContext();
+  Register ListPtr = MI.getOperand(1).getReg();
+  LLT PtrTy = MRI.getType(ListPtr);
+
+  // LstPtr is a pointer to the head of the list. Get the address
+  // of the head of the list.
+  Align PtrAlignment = Align(DL.getABITypeAlign(getTypeForLLT(PtrTy, Ctx)));
+  MachineMemOperand *PtrLoadMMO =
+      MF.getMachineMemOperand(MachinePointerInfo::getUnknownStack(MF),
+                              MachineMemOperand::MOLoad, PtrTy, PtrAlignment);
+  Register HeadOfList = MRI.createGenericVirtualRegister(PtrTy);
+  Register VAList =
+      MIRBuilder.buildLoad(HeadOfList, ListPtr, *PtrLoadMMO).getReg(0);
+
+  const MaybeAlign MA(MI.getOperand(2).getImm());
+  LLT PtrTyAsScalarTy = LLT::scalar(PtrTy.getSizeInBits());
+  if (MA && *MA > TLI.getMinStackArgumentAlignment()) {
+    Register AlignAmt =
+        MIRBuilder.buildConstant(PtrTyAsScalarTy, MA->value() - 1).getReg(0);
+    Register AddDst = MRI.createGenericVirtualRegister(PtrTy);
+    MIRBuilder.buildPtrAdd(AddDst, HeadOfList, AlignAmt);
+    Register Mask =
+        MIRBuilder.buildConstant(PtrTyAsScalarTy, -(int64_t)MA->value())
+            .getReg(0);
+    Register AndDst = MRI.createGenericVirtualRegister(PtrTy);
+    VAList = MIRBuilder.buildPtrMask(AndDst, AddDst, Mask).getReg(0);
+  }
+
+  // Increment the pointer, VAList, to the next vaarg
+  // The list should be bumped by the size of element in the current head of
+  // list.
+  Register Dst = MI.getOperand(0).getReg();
+  LLT Ty = MRI.getType(Dst);
+  Register IncAmt =
+      MIRBuilder
+          .buildConstant(PtrTyAsScalarTy,
+                         DL.getTypeAllocSize(getTypeForLLT(Ty, Ctx)))
+          .getReg(0);
+  Register Succ = MRI.createGenericVirtualRegister(PtrTy);
+  MIRBuilder.buildPtrAdd(Succ, VAList, IncAmt);
+
+  // Store the increment VAList to the legalized pointer
+  MachineMemOperand *StoreMMO =
+      MF.getMachineMemOperand(MachinePointerInfo::getUnknownStack(MF),
+                              MachineMemOperand::MOStore, PtrTy, PtrAlignment);
+  MIRBuilder.buildStore(Succ, ListPtr, *StoreMMO);
+  // Load the actual argument out of the pointer VAList
+  Align EltAlignment = Align(DL.getABITypeAlign(getTypeForLLT(Ty, Ctx)));
+  MachineMemOperand *EltLoadMMO =
+      MF.getMachineMemOperand(MachinePointerInfo::getUnknownStack(MF),
+                              MachineMemOperand::MOLoad, Ty, EltAlignment);
+  MIRBuilder.buildLoad(Dst, VAList, *EltLoadMMO);
+
+  Observer.changedInstr(MI);
+  Observer.erasingInstr(MI);
+  MI.eraseFromParent();
+  return Legalized;
+}
+
 static bool shouldLowerMemFuncForSize(const MachineFunction &MF) {
   // On Darwin, -Os means optimize for size without hurting performance, so
   // only really optimize for size when -Oz (MinSize) is used.
diff --git a/llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp b/llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp
index 9eb5812e024b915..f9e753daff1fbf5 100644
--- a/llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp
+++ b/llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp
@@ -261,6 +261,11 @@ RISCVLegalizerInfo::RISCVLegalizerInfo(const RISCVSubtarget &ST) {
   getActionDefinitionsBuilder({G_FCEIL, G_FFLOOR})
       .libcallFor({s32, s64});
 
+  // va_list must be a pointer, but most sized types are pretty easy to handle
+  // as the destination.
+  getActionDefinitionsBuilder(G_VAARG).lowerForCartesianProduct(
+      {s8, s16, s32, s64, p0}, {p0});
+
   getLegacyLegalizerInfo().computeTables();
 }
 
diff --git a/llvm/test/CodeGen/RISCV/GlobalISel/legalizer/rv32/legalize-vaarg.mir b/llvm/test/CodeGen/RISCV/GlobalISel/legalizer/rv32/legalize-vaarg.mir
new file mode 100644
index 000000000000000..4876e0d5ee96b2f
--- /dev/null
+++ b/llvm/test/CodeGen/RISCV/GlobalISel/legalizer/rv32/legalize-vaarg.mir
@@ -0,0 +1,40 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc -mtriple=riscv32 -run-pass=legalizer %s -o - | FileCheck %s
+
+# On RISC-V, the MinStackArgumentAlignment is 1 and the ABI Alignment for p0 is
+# greater than 1, so we will always generate code to adjust for this alignment.
+
+--- |
+  define void @va_arg() {
+    %va = alloca ptr, align 4
+    %1 = va_arg ptr %va, i32
+    ret void
+  }
+...
+---
+name:            va_arg
+legalized:       false
+tracksRegLiveness: true
+stack:
+  - { id: 0, name: va, type: default, offset: 0, size: 4, alignment: 4,
+      stack-id: default, callee-saved-register: '', callee-saved-restored: true,
+      debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
+body:             |
+  bb.1 (%ir-block.0):
+    ; CHECK-LABEL: name: va_arg
+    ; CHECK: [[FRAME_INDEX:%[0-9]+]]:_(p0) = G_FRAME_INDEX %stack.0.va
+    ; CHECK-NEXT: [[LOAD:%[0-9]+]]:_(p0) = G_LOAD [[FRAME_INDEX]](p0) :: (load (p0))
+    ; CHECK-NEXT: [[C:%[0-9]+]]:_(s32) = G_CONSTANT i32 3
+    ; CHECK-NEXT: [[PTR_ADD:%[0-9]+]]:_(p0) = G_PTR_ADD [[LOAD]], [[C]](s32)
+    ; CHECK-NEXT: [[C1:%[0-9]+]]:_(s32) = G_CONSTANT i32 -4
+    ; CHECK-NEXT: [[PTRTOINT:%[0-9]+]]:_(s32) = G_PTRTOINT [[PTR_ADD]](p0)
+    ; CHECK-NEXT: [[AND:%[0-9]+]]:_(s32) = G_AND [[PTRTOINT]], [[C1]]
+    ; CHECK-NEXT: [[INTTOPTR:%[0-9]+]]:_(p0) = G_INTTOPTR [[AND]](s32)
+    ; CHECK-NEXT: [[C2:%[0-9]+]]:_(s32) = G_CONSTANT i32 4
+    ; CHECK-NEXT: [[PTR_ADD1:%[0-9]+]]:_(p0) = G_PTR_ADD [[INTTOPTR]], [[C2]](s32)
+    ; CHECK-NEXT: G_STORE [[PTR_ADD1]](p0), [[FRAME_INDEX]](p0) :: (store (p0))
+    ; CHECK-NEXT: PseudoRET
+    %0:_(p0) = G_FRAME_INDEX %stack.0.va
+    %1:_(s32) = G_VAARG %0(p0), 4
+    PseudoRET
+...
diff --git a/llvm/test/CodeGen/RISCV/GlobalISel/legalizer/rv64/legalize-vaarg.mir b/llvm/test/CodeGen/RISCV/GlobalISel/legalizer/rv64/legalize-vaarg.mir
new file mode 100644
index 000000000000000..c32f590479c7aa3
--- /dev/null
+++ b/llvm/test/CodeGen/RISCV/GlobalISel/legalizer/rv64/legalize-vaarg.mir
@@ -0,0 +1,40 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc -mtriple=riscv64 -run-pass=legalizer %s -o - | FileCheck %s
+
+# On RISC-V, the MinStackArgumentAlignment is 1 and the ABI Alignment for p0 is
+# greater than 1, so we will always generate code to adjust for this alignment.
+
+--- |
+  define void @va_arg() {
+    %va = alloca ptr, align 8
+    %1 = va_arg ptr %va, i32
+    ret void
+  }
+...
+---
+name:            va_arg
+legalized:       false
+tracksRegLiveness: true
+stack:
+  - { id: 0, name: va, type: default, offset: 0, size: 8, alignment: 4,
+      stack-id: default, callee-saved-register: '', callee-saved-restored: true,
+      debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
+body:             |
+  bb.1 (%ir-block.0):
+    ; CHECK-LABEL: name: va_arg
+    ; CHECK: [[FRAME_INDEX:%[0-9]+]]:_(p0) = G_FRAME_INDEX %stack.0.va
+    ; CHECK-NEXT: [[LOAD:%[0-9]+]]:_(p0) = G_LOAD [[FRAME_INDEX]](p0) :: (load (p0))
+    ; CHECK-NEXT: [[C:%[0-9]+]]:_(s64) = G_CONSTANT i64 3
+    ; CHECK-NEXT: [[PTR_ADD:%[0-9]+]]:_(p0) = G_PTR_ADD [[LOAD]], [[C]](s64)
+    ; CHECK-NEXT: [[C1:%[0-9]+]]:_(s64) = G_CONSTANT i64 -4
+    ; CHECK-NEXT: [[PTRTOINT:%[0-9]+]]:_(s64) = G_PTRTOINT [[PTR_ADD]](p0)
+    ; CHECK-NEXT: [[AND:%[0-9]+]]:_(s64) = G_AND [[PTRTOINT]], [[C1]]
+    ; CHECK-NEXT: [[INTTOPTR:%[0-9]+]]:_(p0) = G_INTTOPTR [[AND]](s64)
+    ; CHECK-NEXT: [[C2:%[0-9]+]]:_(s64) = G_CONSTANT i64 4
+    ; CHECK-NEXT: [[PTR_ADD1:%[0-9]+]]:_(p0) = G_PTR_ADD [[INTTOPTR]], [[C2]](s64)
+    ; CHECK-NEXT: G_STORE [[PTR_ADD1]](p0), [[FRAME_INDEX]](p0) :: (store (p0))
+    ; CHECK-NEXT: PseudoRET
+    %0:_(p0) = G_FRAME_INDEX %stack.0.va
+    %1:_(s32) = G_VAARG %0(p0), 4
+    PseudoRET
+...

@llvmbot
Copy link
Collaborator

llvmbot commented Nov 22, 2023

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

Author: Michael Maitland (michaelmaitland)

Changes

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.


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

5 Files Affected:

  • (modified) llvm/include/llvm/CodeGen/GlobalISel/LegalizerHelper.h (+1)
  • (modified) llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp (+67)
  • (modified) llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp (+5)
  • (added) llvm/test/CodeGen/RISCV/GlobalISel/legalizer/rv32/legalize-vaarg.mir (+40)
  • (added) llvm/test/CodeGen/RISCV/GlobalISel/legalizer/rv64/legalize-vaarg.mir (+40)
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/LegalizerHelper.h b/llvm/include/llvm/CodeGen/GlobalISel/LegalizerHelper.h
index 86d3cb2bedb95b6..350c91ad6fa4f61 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/LegalizerHelper.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/LegalizerHelper.h
@@ -430,6 +430,7 @@ class LegalizerHelper {
   LegalizeResult lowerVectorReduction(MachineInstr &MI);
   LegalizeResult lowerMemcpyInline(MachineInstr &MI);
   LegalizeResult lowerMemCpyFamily(MachineInstr &MI, unsigned MaxLen = 0);
+  LegalizeResult lowerVAArg(MachineInstr &MI);
 };
 
 /// Helper function that creates a libcall to the given \p Name using the given
diff --git a/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp b/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
index dd5577d47f97764..310b71dca37bf3c 100644
--- a/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
@@ -3780,6 +3780,8 @@ LegalizerHelper::lower(MachineInstr &MI, unsigned TypeIdx, LLT LowerHintTy) {
     return lowerTRUNC(MI);
   GISEL_VECREDUCE_CASES_NONSEQ
     return lowerVectorReduction(MI);
+  case G_VAARG:
+    return lowerVAArg(MI);
   }
 }
 
@@ -7865,6 +7867,71 @@ LegalizerHelper::lowerVectorReduction(MachineInstr &MI) {
   return UnableToLegalize;
 }
 
+static Type *getTypeForLLT(LLT Ty, LLVMContext &C);
+
+LegalizerHelper::LegalizeResult LegalizerHelper::lowerVAArg(MachineInstr &MI) {
+  Observer.changingInstr(MI);
+  MachineFunction &MF = *MI.getMF();
+  const DataLayout &DL = MIRBuilder.getDataLayout();
+  LLVMContext &Ctx = MF.getFunction().getContext();
+  Register ListPtr = MI.getOperand(1).getReg();
+  LLT PtrTy = MRI.getType(ListPtr);
+
+  // LstPtr is a pointer to the head of the list. Get the address
+  // of the head of the list.
+  Align PtrAlignment = Align(DL.getABITypeAlign(getTypeForLLT(PtrTy, Ctx)));
+  MachineMemOperand *PtrLoadMMO =
+      MF.getMachineMemOperand(MachinePointerInfo::getUnknownStack(MF),
+                              MachineMemOperand::MOLoad, PtrTy, PtrAlignment);
+  Register HeadOfList = MRI.createGenericVirtualRegister(PtrTy);
+  Register VAList =
+      MIRBuilder.buildLoad(HeadOfList, ListPtr, *PtrLoadMMO).getReg(0);
+
+  const MaybeAlign MA(MI.getOperand(2).getImm());
+  LLT PtrTyAsScalarTy = LLT::scalar(PtrTy.getSizeInBits());
+  if (MA && *MA > TLI.getMinStackArgumentAlignment()) {
+    Register AlignAmt =
+        MIRBuilder.buildConstant(PtrTyAsScalarTy, MA->value() - 1).getReg(0);
+    Register AddDst = MRI.createGenericVirtualRegister(PtrTy);
+    MIRBuilder.buildPtrAdd(AddDst, HeadOfList, AlignAmt);
+    Register Mask =
+        MIRBuilder.buildConstant(PtrTyAsScalarTy, -(int64_t)MA->value())
+            .getReg(0);
+    Register AndDst = MRI.createGenericVirtualRegister(PtrTy);
+    VAList = MIRBuilder.buildPtrMask(AndDst, AddDst, Mask).getReg(0);
+  }
+
+  // Increment the pointer, VAList, to the next vaarg
+  // The list should be bumped by the size of element in the current head of
+  // list.
+  Register Dst = MI.getOperand(0).getReg();
+  LLT Ty = MRI.getType(Dst);
+  Register IncAmt =
+      MIRBuilder
+          .buildConstant(PtrTyAsScalarTy,
+                         DL.getTypeAllocSize(getTypeForLLT(Ty, Ctx)))
+          .getReg(0);
+  Register Succ = MRI.createGenericVirtualRegister(PtrTy);
+  MIRBuilder.buildPtrAdd(Succ, VAList, IncAmt);
+
+  // Store the increment VAList to the legalized pointer
+  MachineMemOperand *StoreMMO =
+      MF.getMachineMemOperand(MachinePointerInfo::getUnknownStack(MF),
+                              MachineMemOperand::MOStore, PtrTy, PtrAlignment);
+  MIRBuilder.buildStore(Succ, ListPtr, *StoreMMO);
+  // Load the actual argument out of the pointer VAList
+  Align EltAlignment = Align(DL.getABITypeAlign(getTypeForLLT(Ty, Ctx)));
+  MachineMemOperand *EltLoadMMO =
+      MF.getMachineMemOperand(MachinePointerInfo::getUnknownStack(MF),
+                              MachineMemOperand::MOLoad, Ty, EltAlignment);
+  MIRBuilder.buildLoad(Dst, VAList, *EltLoadMMO);
+
+  Observer.changedInstr(MI);
+  Observer.erasingInstr(MI);
+  MI.eraseFromParent();
+  return Legalized;
+}
+
 static bool shouldLowerMemFuncForSize(const MachineFunction &MF) {
   // On Darwin, -Os means optimize for size without hurting performance, so
   // only really optimize for size when -Oz (MinSize) is used.
diff --git a/llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp b/llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp
index 9eb5812e024b915..f9e753daff1fbf5 100644
--- a/llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp
+++ b/llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp
@@ -261,6 +261,11 @@ RISCVLegalizerInfo::RISCVLegalizerInfo(const RISCVSubtarget &ST) {
   getActionDefinitionsBuilder({G_FCEIL, G_FFLOOR})
       .libcallFor({s32, s64});
 
+  // va_list must be a pointer, but most sized types are pretty easy to handle
+  // as the destination.
+  getActionDefinitionsBuilder(G_VAARG).lowerForCartesianProduct(
+      {s8, s16, s32, s64, p0}, {p0});
+
   getLegacyLegalizerInfo().computeTables();
 }
 
diff --git a/llvm/test/CodeGen/RISCV/GlobalISel/legalizer/rv32/legalize-vaarg.mir b/llvm/test/CodeGen/RISCV/GlobalISel/legalizer/rv32/legalize-vaarg.mir
new file mode 100644
index 000000000000000..4876e0d5ee96b2f
--- /dev/null
+++ b/llvm/test/CodeGen/RISCV/GlobalISel/legalizer/rv32/legalize-vaarg.mir
@@ -0,0 +1,40 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc -mtriple=riscv32 -run-pass=legalizer %s -o - | FileCheck %s
+
+# On RISC-V, the MinStackArgumentAlignment is 1 and the ABI Alignment for p0 is
+# greater than 1, so we will always generate code to adjust for this alignment.
+
+--- |
+  define void @va_arg() {
+    %va = alloca ptr, align 4
+    %1 = va_arg ptr %va, i32
+    ret void
+  }
+...
+---
+name:            va_arg
+legalized:       false
+tracksRegLiveness: true
+stack:
+  - { id: 0, name: va, type: default, offset: 0, size: 4, alignment: 4,
+      stack-id: default, callee-saved-register: '', callee-saved-restored: true,
+      debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
+body:             |
+  bb.1 (%ir-block.0):
+    ; CHECK-LABEL: name: va_arg
+    ; CHECK: [[FRAME_INDEX:%[0-9]+]]:_(p0) = G_FRAME_INDEX %stack.0.va
+    ; CHECK-NEXT: [[LOAD:%[0-9]+]]:_(p0) = G_LOAD [[FRAME_INDEX]](p0) :: (load (p0))
+    ; CHECK-NEXT: [[C:%[0-9]+]]:_(s32) = G_CONSTANT i32 3
+    ; CHECK-NEXT: [[PTR_ADD:%[0-9]+]]:_(p0) = G_PTR_ADD [[LOAD]], [[C]](s32)
+    ; CHECK-NEXT: [[C1:%[0-9]+]]:_(s32) = G_CONSTANT i32 -4
+    ; CHECK-NEXT: [[PTRTOINT:%[0-9]+]]:_(s32) = G_PTRTOINT [[PTR_ADD]](p0)
+    ; CHECK-NEXT: [[AND:%[0-9]+]]:_(s32) = G_AND [[PTRTOINT]], [[C1]]
+    ; CHECK-NEXT: [[INTTOPTR:%[0-9]+]]:_(p0) = G_INTTOPTR [[AND]](s32)
+    ; CHECK-NEXT: [[C2:%[0-9]+]]:_(s32) = G_CONSTANT i32 4
+    ; CHECK-NEXT: [[PTR_ADD1:%[0-9]+]]:_(p0) = G_PTR_ADD [[INTTOPTR]], [[C2]](s32)
+    ; CHECK-NEXT: G_STORE [[PTR_ADD1]](p0), [[FRAME_INDEX]](p0) :: (store (p0))
+    ; CHECK-NEXT: PseudoRET
+    %0:_(p0) = G_FRAME_INDEX %stack.0.va
+    %1:_(s32) = G_VAARG %0(p0), 4
+    PseudoRET
+...
diff --git a/llvm/test/CodeGen/RISCV/GlobalISel/legalizer/rv64/legalize-vaarg.mir b/llvm/test/CodeGen/RISCV/GlobalISel/legalizer/rv64/legalize-vaarg.mir
new file mode 100644
index 000000000000000..c32f590479c7aa3
--- /dev/null
+++ b/llvm/test/CodeGen/RISCV/GlobalISel/legalizer/rv64/legalize-vaarg.mir
@@ -0,0 +1,40 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc -mtriple=riscv64 -run-pass=legalizer %s -o - | FileCheck %s
+
+# On RISC-V, the MinStackArgumentAlignment is 1 and the ABI Alignment for p0 is
+# greater than 1, so we will always generate code to adjust for this alignment.
+
+--- |
+  define void @va_arg() {
+    %va = alloca ptr, align 8
+    %1 = va_arg ptr %va, i32
+    ret void
+  }
+...
+---
+name:            va_arg
+legalized:       false
+tracksRegLiveness: true
+stack:
+  - { id: 0, name: va, type: default, offset: 0, size: 8, alignment: 4,
+      stack-id: default, callee-saved-register: '', callee-saved-restored: true,
+      debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
+body:             |
+  bb.1 (%ir-block.0):
+    ; CHECK-LABEL: name: va_arg
+    ; CHECK: [[FRAME_INDEX:%[0-9]+]]:_(p0) = G_FRAME_INDEX %stack.0.va
+    ; CHECK-NEXT: [[LOAD:%[0-9]+]]:_(p0) = G_LOAD [[FRAME_INDEX]](p0) :: (load (p0))
+    ; CHECK-NEXT: [[C:%[0-9]+]]:_(s64) = G_CONSTANT i64 3
+    ; CHECK-NEXT: [[PTR_ADD:%[0-9]+]]:_(p0) = G_PTR_ADD [[LOAD]], [[C]](s64)
+    ; CHECK-NEXT: [[C1:%[0-9]+]]:_(s64) = G_CONSTANT i64 -4
+    ; CHECK-NEXT: [[PTRTOINT:%[0-9]+]]:_(s64) = G_PTRTOINT [[PTR_ADD]](p0)
+    ; CHECK-NEXT: [[AND:%[0-9]+]]:_(s64) = G_AND [[PTRTOINT]], [[C1]]
+    ; CHECK-NEXT: [[INTTOPTR:%[0-9]+]]:_(p0) = G_INTTOPTR [[AND]](s64)
+    ; CHECK-NEXT: [[C2:%[0-9]+]]:_(s64) = G_CONSTANT i64 4
+    ; CHECK-NEXT: [[PTR_ADD1:%[0-9]+]]:_(p0) = G_PTR_ADD [[INTTOPTR]], [[C2]](s64)
+    ; CHECK-NEXT: G_STORE [[PTR_ADD1]](p0), [[FRAME_INDEX]](p0) :: (store (p0))
+    ; CHECK-NEXT: PseudoRET
+    %0:_(p0) = G_FRAME_INDEX %stack.0.va
+    %1:_(s32) = G_VAARG %0(p0), 4
+    PseudoRET
+...

llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp Outdated Show resolved Hide resolved
llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp Outdated Show resolved Hide resolved
llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp Outdated Show resolved Hide resolved
llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp Outdated Show resolved Hide resolved
Copy link

github-actions bot commented Nov 28, 2023

✅ With the latest revision this PR passed the C/C++ code formatter.

auto HeadOfList = MIRBuilder.buildLoad(PtrTy, ListPtr, *PtrLoadMMO).getReg(0);
Register VAList = HeadOfList;

const MaybeAlign MA(MI.getOperand(2).getImm());
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you can use Align instead of MaybeAlign based on how IRTranslator::translateVAArg works. It calls getABITypeAlign which returns an Align and then converts that to an integer.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I might have been wrong about that. It looks like the type legalizer in SelectionDAG sets the alignment to 0 to indicate when a ISD::VAARG has been split. We don't have narrowScalar support for G_VAARG yet so we haven't encountered the need for that yet.

Given the way it's used, an alignment of 1 would probably have worked equally well, but maybe 0 made it more obvious to anyone debugging?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

IRTranslator::translateVAArg uses getABITypeAlign which returns an Align and then converts that to an integer.

We're always going to get an Align, not a MaybeAlign. I think we can use Align. SDAG does use 0 to indicate the Hi part of the split, but we are not doing that today in GISel, so I don't think it matters?

Copy link
Collaborator

Choose a reason for hiding this comment

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

IRTranslator::translateVAArg uses getABITypeAlign which returns an Align and then converts that to an integer.

We're always going to get an Align, not a MaybeAlign. I think we can use Align. SDAG does use 0 to indicate the Hi part of the split, but we are not doing that today in GISel, so I don't think it matters?

Don't we need to add splitting to GlobalISel? We have RISC-V tests for it on SelectionDAG.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added clampScalar to this patch which will return UnableToLegalize for now. We will need to implement splitting in a future patch. I think we can use an Align for now and defer the decision on whether it needs to be an Align or Maybe align when there is clampScalar support which is playing with the Align values.

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.
llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp Outdated Show resolved Hide resolved
llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp Outdated Show resolved Hide resolved
llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp Outdated Show resolved Hide resolved
llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp Outdated Show resolved Hide resolved
llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp Outdated Show resolved Hide resolved
llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp Outdated Show resolved Hide resolved
llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp Outdated Show resolved Hide resolved
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 6f9cb9a into llvm:main Dec 8, 2023
4 checks passed
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