[X86] Fix X32 indirect call generation

[hvdijk]

Summary

The check for whether a zero extension was needed was subtly wrong and saw a value that was already 64 bits, so did not extend.

Fixes PR52357.

[hvdijk]

Hm, I should have taken tail call out of the description and test name, this is not limited to tail calls. Then, removing -tailcallopt from the test, it does happen to use a tail call, but that's not the point of the test.

[RKSimon]

llvm/test/CodeGen/X86/call-structfp.ll

4 ↗

(On Diff #383544)

(style) Please can you include a reference to PR52357 somewhere for any future triage/forensics - renaming the method @PR52357 or add a comment, it helps reduce git blame trawling.

[hvdijk]

Renamed function in test.

[hvdijk]

llvm/test/CodeGen/X86/call-structfp.ll

4 ↗

(On Diff #383544)

Sure, done.

[RKSimon]

LGTM

[MaskRay]

llvm/test/CodeGen/X86/call-structfp.ll

1 ↗

(On Diff #384222)

Consider moving this into x32-function_pointer-*.ll

[hvdijk]

llvm/test/CodeGen/X86/call-structfp.ll

1 ↗

(On Diff #384222)

Sure. This comment was after the push, so it'd have to be a new separate change, but I could rename the test to x32-function_pointer-4.ll and add a -fast-isel RUN line like the other x32-function_pointer-*.ll tests if you like.

[MaskRay]

llvm/test/CodeGen/X86/call-structfp.ll

1 ↗

(On Diff #384222)

(When I pushed the comment with "Accept", the revision had been closed.)

May not need a separate file. May just place several cases into one file. It is sometimes easier for readers to follow when placed in one file testing multiple variants.

[hvdijk]

llvm/test/CodeGen/X86/call-structfp.ll

1 ↗

(On Diff #384222)

x32-function_pointer-3.ll is closest to what's tested here since it also involves a function pointer in a struct, so it could be made part of that. I have no strong opinions on whether it should be, but no problem with it. However, that test was not created using update_llc_test_checks.py, and this one is, so one or the other would have to change if they were to be in the same file. This is not difficult, but also not trivial, it requires a little bit of consideration to figure out which way works best; I will take a look at that later.

[hvdijk]

llvm/test/CodeGen/X86/call-structfp.ll

1 ↗

(On Diff #384222)

Created D113157 for this.

llvm/lib/Target/X86/X86ISelLowering.cpp

X86TargetLowering::LowerCall(TargetLowering::CallLoweringInfo &CLI,
   } else if (Callee->getOpcode() == ISD::GlobalAddress ||
              Callee->getOpcode() == ISD::ExternalSymbol) {
     // Lower direct calls to global addresses and external symbols. Setting
     // ForCall to true here has the effect of removing WrapperRIP when possible
     // to allow direct calls to be selected without first materializing the
     // address into a register.
     Callee = LowerGlobalOrExternal(Callee, DAG, /*ForCall=*/true);
   } else if (Subtarget.isTarget64BitILP32() &&
-             Callee->getValueType(0) == MVT::i32) {
+             Callee.getValueType() == MVT::i32) {
     // Zero-extend the 32-bit Callee address into a 64-bit according to x32 ABI
     Callee = DAG.getNode(ISD::ZERO_EXTEND, dl, MVT::i64, Callee);
   }
 
   // Returns a chain & a flag for retval copy to use.
   SDVTList NodeTys = DAG.getVTList(MVT::Other, MVT::Glue);
   SmallVector<SDValue, 8> Ops;
 

llvm/test/CodeGen/X86/tailcall-structfp.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnux32 -tailcallopt | FileCheck %s
+declare { i64, void ()* } @a()
+define void @b() {
+; CHECK-LABEL: b:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    pushq %rax
+; CHECK-NEXT:    .cfi_def_cfa_offset 16
+; CHECK-NEXT:    callq a@PLT
+; CHECK-NEXT:    movl %edx, %eax
+; CHECK-NEXT:    callq *%rax
+; CHECK-NEXT:    popq %rax
+; CHECK-NEXT:    .cfi_def_cfa_offset 8
+; CHECK-NEXT:    retq
+entry:
+  %0 = tail call { i64, void ()* } @a()
+  %1 = extractvalue { i64, void ()* } %0, 1
+  tail call void %1()
+  ret void
+}