[Debug info] Handle endianness when moving debug info for split integer values

[dstenb]

Summary

Take the target's endianness into account when splitting the
debug information in DAGTypeLegalizer::SetExpandedInteger.

This patch fixes so that, for big-endian targets, the fragment
expression corresponding to the high part of a split integer
value is placed at offset 0, in order to correctly represent
the memory address order.

I have attached a PPC32 reproducer where the resulting DWARF
pieces for a 64-bit integer were incorrectly reversed.

[JDevlieghere]

Thanks David!

LGTM, but maybe wait for the other to have a look too before you land this.

PS: When uploading a diff, please include as much context as you can. It makes reviewing a lot easier.

[dstenb]

In D38172#879791, @JDevlieghere wrote:

LGTM, but maybe wait for the other to have a look too before you land this.

Yeah, I will wait.

PS: When uploading a diff, please include as much context as you can. It makes reviewing a lot easier.

I'm sorry about that! Friday afternoon absent-mindedness...

[dblaikie]

test/CodeGen/PowerPC/debuginfo-split-int.ll

18–29 ↗

(On Diff #116328)

Could you simplify this to:

unsigned long long foo();
void bar() {
  volatile unsigned long long result = foo();
}

[aprantl]

Seems plausible.

[dstenb]

Simplified the test case according to dblaikie's comment (thanks!).

[dblaikie]

Sounds good, I think.

[timshen]

Hi,

The test doesn't pass on a llc built without the target "AMDGPU" enabled. To reproduce:

1. Add -DLLVM_TARGETS_TO_BUILD="X86;PowerPC" to the cmake command.
2. re-run cmake and build llc. The test doesn't pass on that llc.

Notice that if we define LLVM_TARGETS_TO_BUILD as "X86;PowerPC;AMDGPU", the test passes again.

I think this is related to [1], but I don't understand why "isel" defined in AMDGPU gets picked up by a PPC32 through "-stop-after=isel".

[1] https://github.com/llvm-mirror/llvm/blob/7287fcb5d580e8ac0b860ddd4da03dc395795fa0/lib/Target/AMDGPU/AMDGPUISelDAGToDAG.cpp#L237

[bjope]

In D38172#886397, @timshen wrote:

Hi,

The test doesn't pass on a llc built without the target "AMDGPU" enabled. To reproduce:

1. Add -DLLVM_TARGETS_TO_BUILD="X86;PowerPC" to the cmake command.
2. re-run cmake and build llc. The test doesn't pass on that llc.

Notice that if we define LLVM_TARGETS_TO_BUILD as "X86;PowerPC;AMDGPU", the test passes again.

I think this is related to [1], but I don't understand why "isel" defined in AMDGPU gets picked up by a PPC32 through "-stop-after=isel".

[1] https://github.com/llvm-mirror/llvm/blob/7287fcb5d580e8ac0b860ddd4da03dc395795fa0/lib/Target/AMDGPU/AMDGPUISelDAGToDAG.cpp#L237

Ok, thanks for the revert Tim. I'll reapply this later today by using -stop-before=expand-isel-pseudos instead of -stop-after=isel.
(a bummer that AMDGPU is using such a general name as "isel" when registering the target specific pass... kind of fooled us to think that we could stop after "isel" for any target...)

llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp

assert(Lo.getValueType() ==
          TLI.getTypeToTransformTo(*DAG.getContext(), Op.getValueType()) &&
          Hi.getValueType() == Lo.getValueType() &&
          "Invalid type for expanded integer");
   // Lo/Hi may have been newly allocated, if so, add nodeid's as relevant.
   AnalyzeNewValue(Lo);
   AnalyzeNewValue(Hi);
 
   // Transfer debug values.
+  if (DAG.getDataLayout().isBigEndian()) {
+    transferDbgValues(DAG, Op, Hi, 0);
+    transferDbgValues(DAG, Op, Lo, Hi.getValueSizeInBits());
+  } else {
     transferDbgValues(DAG, Op, Lo, 0);
     transferDbgValues(DAG, Op, Hi, Lo.getValueSizeInBits());
+  }
 
   // Remember that this is the result of the node.
   std::pair<SDValue, SDValue> &Entry = ExpandedIntegers[Op];
   assert(!Entry.first.getNode() && "Node already expanded");
   Entry.first = Lo;
   Entry.second = Hi;
 }
 

llvm/trunk/test/CodeGen/PowerPC/debuginfo-split-int.ll

+; RUN: llc < %s -stop-after=isel -o - | FileCheck %s
+
+source_filename = "foo.c"
+target datalayout = "E-m:e-p:32:32-i64:64-n32"
+target triple = "ppc32"
+
+; Verify that, when handling split-up integers, the
+; transferring of debug info takes the endianness
+; into consideration.
+;
+; The fragment expression at offset 0 should correspond
+; to the high part of the value on big-endian targets.
+
+; This basis of this ll file was created by running:
+;   clang --target=powerpc -O1 -S -g -emit-llvm foo.c
+;
+; with foo.c being the program:
+;   unsigned long long foo(void);
+;   void bar() {
+;     volatile unsigned long long result = foo();
+;   }
+;
+; This file is a slight tweak of that output, with irrelevant
+; lifetime intrinsics, metadata, and debug info being removed.
+
+; CHECK: [[DL:![0-9]+]] = !DILocalVariable(name: "result"
+;
+; High 32 bits in R3, low 32 bits in R4
+; CHECK: %0 = COPY %r3
+; CHECK: DBG_VALUE debug-use %0, debug-use _, [[DL]], !DIExpression(DW_OP_LLVM_fragment, 0, 32)
+; CHECK: %1 = COPY %r4
+; CHECK: DBG_VALUE debug-use %1, debug-use _, [[DL]], !DIExpression(DW_OP_LLVM_fragment, 32, 32)
+define void @bar() local_unnamed_addr #0 !dbg !6 {
+  %1 = alloca i64, align 8
+  %2 = tail call i64 @foo()
+  tail call void @llvm.dbg.value(metadata i64 %2, metadata !10, metadata !DIExpression()), !dbg !13
+  store volatile i64 %2, i64* %1, align 8
+  ret void
+}
+
+declare i64 @foo() local_unnamed_addr
+
+; Function Attrs: nounwind readnone speculatable
+declare void @llvm.dbg.value(metadata, metadata, metadata) #1
+
+attributes #0 = { nounwind }
+attributes #1 = { nounwind readnone speculatable }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4}
+!llvm.ident = !{!5}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 6.0.0", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2)
+!1 = !DIFile(filename: "foo.c", directory: "/")
+!2 = !{}
+!3 = !{i32 2, !"Dwarf Version", i32 4}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{!"clang version 6.0.0"}
+!6 = distinct !DISubprogram(name: "bar", scope: !1, file: !1, line: 2, type: !7, isLocal: false, isDefinition: true, scopeLine: 2, isOptimized: true, unit: !0, variables: !9)
+!7 = !DISubroutineType(types: !8)
+!8 = !{null}
+!9 = !{!10}
+!10 = !DILocalVariable(name: "result", scope: !6, file: !1, line: 3, type: !11)
+!11 = !DIDerivedType(tag: DW_TAG_volatile_type, baseType: !12)
+!12 = !DIBasicType(name: "long long unsigned int", size: 64, encoding: DW_ATE_unsigned)
+!13 = !DILocation(line: 3, column: 31, scope: !6)