[GlobalISel] Assign the correct debug location when combining G_SEXT/G_ANYEXT/G_ZEXT

[paquette]

The code looks fine to me, but I think the testcase can be tidied up a bit.

llvm/test/CodeGen/AArch64/GlobalISel/combine-ext-debugloc.mir

1

Should this have -verify-machineinstrs?

8–9

Probably can just remove this?

37–38

Would be nice to replace the paths with dummy paths?

42

Would be nice to replace this path with a dummy path?

51–58

I think you can remove most of these?

59–73

I think you can remove this too and the test should still work.

75–93

I think you can remove all of this?

[davide]

llvm/test/CodeGen/AArch64/GlobalISel/combine-ext-debugloc.mir

8–9

Just afraid it might freak out when run on linux, but I guess passing -mtriple explicitly overrides the setting?

[vsk]

I'm not sure the G_SEXT case is covered -- could you add a sext(trunc x) pattern to the MIR?

Also, I think you can delete the IR contents of @main, those shouldn't be needed to construct the MIR.

[vsk]

llvm/test/CodeGen/AArch64/GlobalISel/combine-ext-debugloc.mir

8–9

IME the target data layout & triple sometimes do matter, as some builders may be configured with different assumptions about what these are by default.

[davide]

Addressed Jessica's feedback.

[davide]

In D79031#2008718, @vsk wrote:

I'm not sure the G_SEXT case is covered -- could you add a sext(trunc x) pattern to the MIR?

Good point, I'll revert that bit & do that in a subsequent commit.

Also, I think you can delete the IR contents of @main, those shouldn't be needed to construct the MIR.

That didn't work.

[dsanders]

LGTM but double check -verify-machineinstrs works with -run-pass

llvm/test/CodeGen/AArch64/GlobalISel/combine-ext-debugloc.mir

1

Also, does it actually run? It used to be the case that -start-before=X -stop-after=X ran it but -run-pass=X didn't. I don't remember off-hand whether that got fixed

41–51

The 'registers' section can be safely removed

[davide]

I guess I addressed everybody's comments.

commit 0ed276bb08a476a4d44d96afad7c077876a8c0ed (HEAD -> master, origin/master, origin/HEAD)
Author: Davide Italiano <[e-mail]>
Date: Tue Apr 28 13:09:28 2020 -0700

[GlobalISel] Assign the correct debug location when combining G_ANYEXT/G_ZEXT

<rdar://problem/62535712>

[davide]

llvm/test/CodeGen/AArch64/GlobalISel/combine-ext-debugloc.mir

41–51

The test fails if I do.

llvm/include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h

LegalizationArtifactCombiner(MachineIRBuilder &B, MachineRegisterInfo &MRI,
                     const LegalizerInfo &LI)
       : Builder(B), MRI(MRI), LI(LI) {}
 
   bool tryCombineAnyExt(MachineInstr &MI,
                         SmallVectorImpl<MachineInstr *> &DeadInsts,
                         SmallVectorImpl<Register> &UpdatedDefs) {
     assert(MI.getOpcode() == TargetOpcode::G_ANYEXT);
 
-    Builder.setInstr(MI);
+    Builder.setInstrAndDebugLoc(MI);
     Register DstReg = MI.getOperand(0).getReg();
     Register SrcReg = lookThroughCopyInstrs(MI.getOperand(1).getReg());
 
     // aext(trunc x) - > aext/copy/trunc x
     Register TruncSrc;
     if (mi_match(SrcReg, MRI, m_GTrunc(m_Reg(TruncSrc)))) {
       LLVM_DEBUG(dbgs() << ".. Combine MI: " << MI;);
       Builder.buildAnyExtOrTrunc(DstReg, TruncSrc);
public:
   }
 
   bool tryCombineZExt(MachineInstr &MI,
                       SmallVectorImpl<MachineInstr *> &DeadInsts,
                       SmallVectorImpl<Register> &UpdatedDefs,
                       GISelObserverWrapper &Observer) {
     assert(MI.getOpcode() == TargetOpcode::G_ZEXT);
 
-    Builder.setInstr(MI);
+    Builder.setInstrAndDebugLoc(MI);
     Register DstReg = MI.getOperand(0).getReg();
     Register SrcReg = lookThroughCopyInstrs(MI.getOperand(1).getReg());
 
     // zext(trunc x) - > and (aext/copy/trunc x), mask
     Register TruncSrc;
     if (mi_match(SrcReg, MRI, m_GTrunc(m_Reg(TruncSrc)))) {
       LLT DstTy = MRI.getType(DstReg);
       if (isInstUnsupported({TargetOpcode::G_AND, {DstTy}}) ||
bool tryCombineZExt(MachineInstr &MI,
     return tryFoldImplicitDef(MI, DeadInsts, UpdatedDefs);
   }
 
   bool tryCombineSExt(MachineInstr &MI,
                       SmallVectorImpl<MachineInstr *> &DeadInsts,
                       SmallVectorImpl<Register> &UpdatedDefs) {
     assert(MI.getOpcode() == TargetOpcode::G_SEXT);
 
-    Builder.setInstr(MI);
+    Builder.setInstrAndDebugLoc(MI);
     Register DstReg = MI.getOperand(0).getReg();
     Register SrcReg = lookThroughCopyInstrs(MI.getOperand(1).getReg());
 
     // sext(trunc x) - > (sext_inreg (aext/copy/trunc x), c)
     Register TruncSrc;
     if (mi_match(SrcReg, MRI, m_GTrunc(m_Reg(TruncSrc)))) {
       LLT DstTy = MRI.getType(DstReg);
       if (isInstUnsupported({TargetOpcode::G_SEXT_INREG, {DstTy}}))

llvm/test/CodeGen/AArch64/GlobalISel/combine-ext-debugloc.mir

+# RUN: llc -O0 -mtriple aarch64-- -run-pass=legalizer %s -o - | FileCheck %s
+
+# Check that when we combine SEXT/ZEXT/ANYEXT we assign the correct location.
+# CHECK: !8 = !DILocation(line: 23, column: 5, scope: !4)
+# CHECK:  G_AND %16, %15, debug-location !8
+
+--- |
+  target datalayout = "e-m:o-i64:64-i128:128-n32:64-S128"
+  target triple = "arm64-apple-ios13.0.0"
+  
+  @.str = external dso_local unnamed_addr constant [4 x i8], align 1
+  
+  define void @main() #0 personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*) !dbg !4 {
+  entry:
+    %tobool = trunc i8 undef to i1
+    %conv = zext i1 %tobool to i32
+    call void (i8*, ...) @printf(i8* getelementptr inbounds ([4 x i8], [4 x i8]* @.str, i64 0, i64 0), i32 %conv) #2, !dbg !8
+    %0 = load i32, i32* undef, align 4, !dbg !9
+    call void (i8*, ...) @printf(i8* getelementptr inbounds ([4 x i8], [4 x i8]* @.str, i64 0, i64 0), i32 %0) #2
+    ret void
+  }
+  
+  declare void @printf(i8*, ...) #0
+  
+  declare i32 @__gxx_personality_v0(...)
+  
+  ; Function Attrs: nounwind
+  declare void @llvm.stackprotector(i8*, i8**) #1
+  
+  attributes #0 = { "use-soft-float"="false" }
+  attributes #1 = { nounwind }
+  attributes #2 = { nobuiltin "no-builtins" }
+  
+  !llvm.dbg.cu = !{!0}
+  !llvm.module.flags = !{!2, !3}
+  
+  !0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang version 10.0.0", isOptimized: false, runtimeVersion: 0, emissionKind: LineTablesOnly, nameTableKind: None, sysroot: "/Applications/Xcode2.app/Contents/Developer/Platforms/iPhoneOS.platform/Developer/SDKs/iPhoneOS13.0.sdk", sdk: "iPhoneOS13.0.sdk")
+  !1 = !DIFile(filename: "/Users/davide/stash_work/llvm-project/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/variant/main.cpp", directory: "/Users/davide")
+  !2 = !{i32 2, !"Debug Info Version", i32 3}
+  !3 = !{i32 7, !"PIC Level", i32 2}
+  !4 = distinct !DISubprogram(name: "main", scope: !5, file: !5, line: 19, type: !6, scopeLine: 20, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0)
+  !5 = !DIFile(filename: "stash_work/llvm-project/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/variant/main.cpp", directory: "/Users/davide")
+  !6 = !DISubroutineType(types: !7)
+  !7 = !{}
+  !8 = !DILocation(line: 23, column: 5, scope: !4)
+  !9 = !DILocation(line: 36, column: 21, scope: !4)
+
+...
+---
+name:            main
+alignment:       4
+exposesReturnsTwice: false
+legalized:       false
+regBankSelected: false
+selected:        false
+failedISel:      false
+tracksRegLiveness: true
+hasWinCFI:       false
+registers:
+  - { id: 0, class: _, preferred-register: '' }
+  - { id: 1, class: _, preferred-register: '' }
+  - { id: 2, class: _, preferred-register: '' }
+  - { id: 3, class: _, preferred-register: '' }
+  - { id: 4, class: _, preferred-register: '' }
+  - { id: 5, class: _, preferred-register: '' }
+  - { id: 6, class: _, preferred-register: '' }
+  - { id: 7, class: _, preferred-register: '' }
+  - { id: 8, class: _, preferred-register: '' }
+  - { id: 9, class: _, preferred-register: '' }
+  - { id: 10, class: _, preferred-register: '' }
+  - { id: 11, class: _, preferred-register: '' }
+  - { id: 12, class: _, preferred-register: '' }
+  - { id: 13, class: _, preferred-register: '' }
+liveins:         []
+frameInfo:
+  isFrameAddressTaken: false
+  isReturnAddressTaken: false
+  hasStackMap:     false
+  hasPatchPoint:   false
+  stackSize:       0
+  offsetAdjustment: 0
+  maxAlignment:    8
+  adjustsStack:    false
+  hasCalls:        false
+  stackProtector:  ''
+  maxCallFrameSize: 4294967295
+  cvBytesOfCalleeSavedRegisters: 0
+  hasOpaqueSPAdjustment: false
+  hasVAStart:      false
+  hasMustTailInVarArgFunc: false
+  localFrameSize:  0
+  savePoint:       ''
+  restorePoint:    ''
+fixedStack:      []
+stack:           []
+callSites:       []
+constants:       []
+machineFunctionInfo: {}
+body:             |
+  bb.1.entry:
+    %0:_(s8) = G_IMPLICIT_DEF
+    %4:_(p0) = G_GLOBAL_VALUE @.str, debug-location !DILocation(line: 0, scope: !4)
+    %10:_(p0) = G_IMPLICIT_DEF debug-location !DILocation(line: 0, scope: !4)
+    %1:_(s1) = G_TRUNC %0(s8)
+    %2:_(s32) = G_ZEXT %1(s1)
+    ADJCALLSTACKDOWN 8, 0, implicit-def $sp, implicit $sp, debug-location !8
+    $x0 = COPY %4(p0), debug-location !8
+    %5:_(p0) = COPY $sp, debug-location !8
+    %6:_(s64) = G_CONSTANT i64 0
+    %7:_(p0) = G_PTR_ADD %5, %6(s64), debug-location !8
+    %8:_(s64) = G_ANYEXT %2(s32), debug-location !8
+    G_STORE %8(s64), %7(p0), debug-location !8 :: (store 8 into stack, align 1)
+    BL @printf, csr_aarch64_aapcs, implicit-def $lr, implicit $sp, implicit $x0, debug-location !8
+    ADJCALLSTACKUP 8, 0, implicit-def $sp, implicit $sp, debug-location !8
+    %13:_(s64) = G_LOAD %10(p0), debug-location !9 :: (load 4 from `i32* undef`)
+    ADJCALLSTACKDOWN 8, 0, implicit-def $sp, implicit $sp
+    $x0 = COPY %4(p0)
+    %11:_(p0) = COPY $sp
+    %12:_(p0) = G_PTR_ADD %11, %6(s64)
+    G_STORE %13(s64), %12(p0) :: (store 8 into stack, align 1)
+    BL @printf, csr_aarch64_aapcs, implicit-def $lr, implicit $sp, implicit $x0
+    ADJCALLSTACKUP 8, 0, implicit-def $sp, implicit $sp
+    RET_ReallyLR
+
+...