[PATCH] D31010: Fix PR32288

David Blaikie via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 15 16:43:25 PDT 2017


dblaikie accepted this revision.
dblaikie added a comment.
This revision is now accepted and ready to land.

Looks OK :)



================
Comment at: lib/CodeGen/AsmPrinter/DwarfExpression.cpp:290-291
+  // Emit any outstanding DW_OP_piece operations to mask out subregisters.
+  if (SubRegisterSizeInBits == 0)
+    return;
+  // Don't emit a DW_OP_piece for a subregister at offset 0.
----------------
Is this ever true? What does that mean? (I guess it means there's no sub register at work? But that'd be handled by the below case too, right?)

Is this to improve legibility, then?


================
Comment at: test/DebugInfo/X86/PR26148.ll:22
 ;
-; CHECK: 0x00000025: Beginning address offset: 0x0000000000000004
+; CHECK:             Beginning address offset: 0x0000000000000004
 ; CHECK:                Ending address offset: 0x0000000000000004
----------------
That's a bit confusing/interesting. (wonder what the rest of that location description now contains)


================
Comment at: test/DebugInfo/X86/dw_op_minus_direct.ll:11-12
 ; CHECK:    Ending address offset: 0x0000000000000004
-; CHECK:     Location description: 50 10 01 1c 93 04
-;                                  rax, constu 0x00000001, minus, piece 0x00000004
+; CHECK:     Location description: 50 10 01 1c
+;                                  rax, constu 0x00000001, minus
 source_filename = "minus.c"
----------------
This one's interesting - because the register's not at the top level.

I would've expected the piece(4) would go against rax first, before the "- 1" & /could/ be relevant (a consumer might not know whether a register used in a complex location expression is the size of the destination value or not, etc)? But I guess it's probably OK?


https://reviews.llvm.org/D31010





More information about the llvm-commits mailing list