[llvm] [SelectionDAGBuilder] Remove the added base offset in LowerFormalArguments(). (PR #170732)

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-selectiondag

Author: Jonas Paulsson (JonPsson1)

<details>
<summary>Changes</summary>

LowerCallTo() and LowerArguments() are both providing the PartOffset field for
each split argument part. As these two methods are intended to work together,
it seems ideal that both provide the same offsets. However, currently
LowerArguments() sets the offset from the beginning of a struct, while
LowerCallTo() sets it relative to the first split part.

Removing the PartBase variable in LowerArguments() (and also the SystemZ
workaround recently added) does not seem to break any tests.

Is there a reason to have that around, or would this patch make sense at this point?

@<!-- -->tstellar @<!-- -->arsenm @<!-- -->nikic @<!-- -->momchil-velikov @<!-- -->aeubanks @<!-- -->uweigand 

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


2 Files Affected:

- (modified) llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp (+1-3) 
- (modified) llvm/lib/Target/SystemZ/SystemZISelLowering.cpp (+1-8) 


``````````diff
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
index 09a0673bfe1bb..4fc06bd4244ed 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -11848,7 +11848,6 @@ void SelectionDAGISel::LowerArguments(const Function &F) {
     SmallVector<Type *, 4> Types;
     ComputeValueTypes(DAG.getDataLayout(), Arg.getType(), Types);
     bool isArgValueUsed = !Arg.use_empty();
-    unsigned PartBase = 0;
     Type *FinalType = Arg.getType();
     if (Arg.hasAttribute(Attribute::ByVal))
       FinalType = Arg.getParamByValType();
@@ -11967,7 +11966,7 @@ void SelectionDAGISel::LowerArguments(const Function &F) {
         // return values.
         ISD::InputArg MyFlags(
             Flags, RegisterVT, VT, ArgTy, isArgValueUsed, ArgNo,
-            PartBase + i * RegisterVT.getStoreSize().getKnownMinValue());
+            i * RegisterVT.getStoreSize().getKnownMinValue());
         if (NumRegs > 1 && i == 0)
           MyFlags.Flags.setSplit();
         // if it isn't first piece, alignment must be 1
@@ -11980,7 +11979,6 @@ void SelectionDAGISel::LowerArguments(const Function &F) {
       }
       if (NeedsRegBlock && Value == NumValues - 1)
         Ins[Ins.size() - 1].Flags.setInConsecutiveRegsLast();
-      PartBase += VT.getStoreSize().getKnownMinValue();
     }
   }
 
diff --git a/llvm/lib/Target/SystemZ/SystemZISelLowering.cpp b/llvm/lib/Target/SystemZ/SystemZISelLowering.cpp
index 2511d08a6d0ef..d803bc37f8fc0 100644
--- a/llvm/lib/Target/SystemZ/SystemZISelLowering.cpp
+++ b/llvm/lib/Target/SystemZ/SystemZISelLowering.cpp
@@ -2099,17 +2099,10 @@ SDValue SystemZTargetLowering::LowerFormalArguments(
       MVT PartVT;
       unsigned NumParts;
       if (analyzeArgSplit(Ins, ArgLocs, I, PartVT, NumParts)) {
-        // TODO: It is strange that while LowerCallTo() sets the PartOffset
-        // relative to the first split part LowerArguments() sets the offset
-        // from the beginning of the struct. So with {i32, i256}, the
-        // PartOffset for the i256 parts are differently handled. Try to
-        // remove that difference and use PartOffset directly here (instead
-        // of SplitBaseOffs).
-        unsigned SplitBaseOffs = Ins[I].PartOffset;
         for (unsigned PartIdx = 1; PartIdx < NumParts; ++PartIdx) {
           ++I;
           CCValAssign &PartVA = ArgLocs[I];
-          unsigned PartOffset = Ins[I].PartOffset - SplitBaseOffs;
+          unsigned PartOffset = Ins[I].PartOffset;
           SDValue Address = DAG.getNode(ISD::ADD, DL, PtrVT, ArgValue,
                                         DAG.getIntPtrConstant(PartOffset, DL));
           InVals.push_back(DAG.getLoad(PartVA.getValVT(), DL, Chain, Address,

``````````

</details>


https://github.com/llvm/llvm-project/pull/170732