fix callsite attribute copying in statepoint lowering

[carnaval]

Summary

The loop copying attributes from the statepoint to the lowered call site was missing an increment.

[sanjoy]

lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

7180

Can you please upload with full context, like you did for the other patches?

test/CodeGen/X86/statepoint-attributes.ll

2

You don't need the 2>&1.

11

Use full sentences -- start with uppercase, end with period etc.

[carnaval]

add context, reword test comment

[sanjoy]

lgtm with a nit inline

test/CodeGen/X86/statepoint-attributes.ll

19

I'd also add a positive test (i.e. ; CHECK: mov ... {{{[0-9]+}}}(%rsp); CHECK: callq or something like that).

Also, how about adding this test to test/CodeGen/X86/deopt-bundles.ll?

[Eugene.Zelenko]

Looks like patch was not committed.

lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

void SelectionDAGBuilder::populateCallLoweringInfo(
     TargetLowering::CallLoweringInfo &CLI, ImmutableCallSite CS,
     unsigned ArgIdx, unsigned NumArgs, SDValue Callee, Type *ReturnTy,
     bool IsPatchPoint) {
   TargetLowering::ArgListTy Args;
   Args.reserve(NumArgs);
 
   // Populate the argument list.
   // Attributes for args start at offset 1, after the return attribute.
-  for (unsigned ArgI = ArgIdx, ArgE = ArgIdx + NumArgs, AttrI = ArgIdx + 1;
-       ArgI != ArgE; ++ArgI) {
+  for (unsigned ArgI = ArgIdx, ArgE = ArgIdx + NumArgs, AttrI = ArgIdx+1;
+       ArgI != ArgE; ++ArgI, ++AttrI) {
     const Value *V = CS->getOperand(ArgI);
 
     assert(!V->getType()->isEmptyTy() && "Empty type passed to intrinsic.");
 
     TargetLowering::ArgListEntry Entry;
     Entry.Node = getValue(V);
     Entry.Ty = V->getType();
     Entry.setAttributes(&CS, AttrI);

test/CodeGen/X86/statepoint-attributes.ll

+; RUN: llc < %s | FileCheck %s
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+%struct.struct6 = type { i64, i64, i64 }
+
+@f.I = private unnamed_addr constant %struct.struct6 { i64 1, i64 2, i64 3 }, align 8
+
+define i64 @f() {
+  ; Here we check that the statepoint lowering carries the attributes of the original call site correctly.
+  ; If it did not the byval attribute could be lost and, following the ABI, we would use %rdx to pass
+  ; the third argument.
+  ; With the byval argument set the structure is passed on the stack and only two of the integer-passing
+  ; registers are used.
+  ; CHECK-NOT: edx
+  ; CHECK-NOT: rdx
+  ; CHECK-NOT: dx
+  ; CHECK-NOT: dl
+  %O = alloca %struct.struct6, align 8
+  %1 = bitcast %struct.struct6* %O to i8*
+  call void @test_6(%struct.struct6* nonnull sret %O, %struct.struct6* byval nonnull align 8 @f.I, i64 4) [ "deopt"(i8* %1) ]
+  %2 = getelementptr inbounds %struct.struct6, %struct.struct6* %O, i64 0, i32 2
+  %3 = load i64, i64* %2, align 8
+  ret i64 %3
+}
+
+declare void @test_6(%struct.struct6* sret, %struct.struct6* byval align 8, i64)