[PATCH] D125680: Correctly legalise stackmap operands

[Edd Barrett via Phabricator via llvm-commits]

vext01 added a comment.

To ease reviewing, here's what changed since the last version:

  diff --git a/llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp b/llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp
  index 64d92e60f5de..7ed2808600cf 100644
  --- a/llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp
  +++ b/llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp
  @@ -5515,8 +5515,12 @@ SDValue DAGTypeLegalizer::ExpandIntOp_STACKMAP(SDNode *N, unsigned OpNo) {
           DAG.getTargetConstant(StackMaps::ConstantOp, DL, MVT::i64));
       NewOps.push_back(DAG.getTargetConstant(CN->getZExtValue(), DL, Ty));
     } else {
  -    // FIXME: https://github.com/llvm/llvm-project/issues/26431
  -    DAG.getContext()->emitError("Can't expand stackmap operand");
  +    // FIXME: There are a couple of problems with expanding non-constants for
  +    // stackmaps:
  +    //  - https://github.com/llvm/llvm-project/issues/26431
  +    //  - https://github.com/llvm/llvm-project/issues/55957
  +    DAG.getContext()->emitError(
  +        "expanding this stackmap operand is unimplemented");
     }
   
     // Copy remaining operands.
  diff --git a/llvm/test/CodeGen/X86/selectiondag-stackmap-legalize.ll b/llvm/test/CodeGen/X86/selectiondag-stackmap-legalize.ll
  index a93ffac1e1bb..80d95e3b66a5 100644
  --- a/llvm/test/CodeGen/X86/selectiondag-stackmap-legalize.ll
  +++ b/llvm/test/CodeGen/X86/selectiondag-stackmap-legalize.ll
  @@ -29,7 +29,7 @@
   ;     CHECK-NEXT:   .long {{.*}}
   ;     CHECK-NEXT:   .short {{.*}}
   ;     NumLocations
  -;     CHECK-NEXT:   .short 5
  +;     CHECK-NEXT:   .short 7
   ;     Location[NumLocations]
   ;       Location[0]
   ;         CHECK-NEXT: .byte   1
  @@ -48,7 +48,7 @@
   ;       Location[2]
   ;         CHECK-NEXT: .byte   1
   ;         CHECK-NEXT: .byte   0
  -;         CHECK-NEXT: .short  16
  +;         CHECK-NEXT: .short  1
   ;         CHECK-NEXT: .short  {{.*}}
   ;         CHECK-NEXT: .short  0
   ;         CHECK-NEXT: .long   0
  @@ -60,26 +60,51 @@
   ;         CHECK-NEXT: .short  0
   ;         CHECK-NEXT: .long   0
   ;       Location[4]
  +;         CHECK-NEXT: .byte   1
  +;         CHECK-NEXT: .byte   0
  +;         CHECK-NEXT: .short  16
  +;         CHECK-NEXT: .short  {{.*}}
  +;         CHECK-NEXT: .short  0
  +;         CHECK-NEXT: .long   0
  +;       Location[5]
   ;         CHECK-NEXT: .byte   4
   ;         CHECK-NEXT: .byte   0
   ;         CHECK-NEXT: .short  8
   ;         CHECK-NEXT: .short  {{.*}}
   ;         CHECK-NEXT: .short  0
   ;         CHECK-NEXT: .long   66
  +;       Location[4]
  +;         CHECK-NEXT: .byte   1
  +;         CHECK-NEXT: .byte   0
  +;         CHECK-NEXT: .short  4
  +;         CHECK-NEXT: .short  {{.*}}
  +;         CHECK-NEXT: .short  0
  +;         CHECK-NEXT: .long   0
  +
  + at p32 = external global i8 addrspace(270)*
   
   declare void @llvm.experimental.stackmap(i64, i32, ...)
   
   define dso_local i32 @main(i32 %argc, i8** %argv) {
   entry:
  -  %intreg = icmp eq i32 %argc, 5
  +  %i1reg = icmp eq i32 %argc, 5
  +  %i7reg = zext i1 %i1reg to i7
  +  %i128reg = zext i1 %i1reg to i128
     %halfreg = sitofp i32 %argc to half
  +  %ptr32 = load i8 addrspace(270)*, i8 addrspace(270)** @p32
     call void (i64, i32, ...) @llvm.experimental.stackmap(
       i64 0,
       i32 0,
  -    i1 %intreg,
  +    i1 %i1reg,
       i7 22,
  +    i7 %i7reg,
       half 1.0,
       half %halfreg,
  -    i128 66)
  +    i128 66,
  +    ; FIXME: test non-constant i128 once these are fixed:
  +    ;  - https://github.com/llvm/llvm-project/issues/26431
  +    ;  - https://github.com/llvm/llvm-project/issues/55957
  +    ;i128 %i128reg
  +    i8 addrspace(270)* %ptr32)
     ret i32 0
   }



================
Comment at: llvm/test/CodeGen/X86/selectiondag-stackmap-legalize.ll:83
+    half %halfreg,
+    i128 66)
+  ret i32 0
----------------
arsenm wrote:
> For a general test, a non-register argument may be more helpful
I've added the missing non-constant i7, and I would have done the same for `i128`, but that's already broken in llvm before my change and my change doesn't fix it either. That can be another diff to fix that.

Also added a 32-bit address space pointer into the test.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125680/new/

https://reviews.llvm.org/D125680