[PATCH] D48468: [SelectionDAG] Remove debug locations from ConstantSD(FP)Nodes

[Vedant Kumar via Phabricator via llvm-commits]

vsk added a comment.

>> From @yroux:
> 
> Hi Vedant,
> 
> This patch broke ARM bots since expected line info is missing in
>  address sanitizer output for null_redef.cc testcase, notice that when
>  the test is compiled in thumb mode the line info is still present.
> 
> logs available at:
> 
> http://lab.llvm.org:8011/builders/clang-cmake-armv7-full/builds/1203/steps/ninja%20check%201/logs/FAIL%3A%20AddressSanitizer-armhf-linux%3A%3Anull_deref.cc
> 
> Thanks
> Yvan

Phab didn't pick up my reply from the mailing list, so just to recap: thanks for bringing this to my attention -- I've relaxed the test and the bot has recovered.

In https://reviews.llvm.org/D48468#1147627, @petpav01 wrote:

> Hi Vedant,
>
> This change also made debug information for the following example slightly worse:
>
>   $ cat test.c
>   void bar(void);
>   int foo(void) {
>     bar();
>     int i = 2;
>     return i;
>   }
>   $ clang -target armv7a-none-eabi -g -c test.c main.c
>   $ llvm-objdump -S test.o
>  
>   test.o: file format ELF32-arm-little
>  
>   Disassembly of section .text:
>   foo:
>   ; int foo(void) {
>          0:       00 48 2d e9     push    {r11, lr}
>          4:       0d b0 a0 e1     mov     r11, sp
>          8:       08 d0 4d e2     sub     sp, sp, #8
>   ; bar();
>          c:       fe ff ff eb     bl      #-8 <foo+0xc>
>         10:       02 e0 a0 e3     mov     lr, #2
>   ; int i = 2;
>         14:       04 e0 8d e5     str     lr, [sp, #4]
>   ; return i;
>         18:       04 00 9d e5     ldr     r0, [sp, #4]
>         1c:       0b d0 a0 e1     mov     sp, r11
>         20:       00 88 bd e8     pop     {r11, pc}
>
>
> Instruction `mov lr, #2` is recorded as part of line 3 (`bar();`) but previously it was more accurately accounted as line 4 (`int i = 2;`).
>
> Previous behaviour:
>
>   $ llvm-objdump -S test.o
>  
>   test.o: file format ELF32-arm-little
>  
>   Disassembly of section .text:
>   foo:
>   ; int foo(void) {
>          0:       00 48 2d e9     push    {r11, lr}
>          4:       0d b0 a0 e1     mov     r11, sp
>          8:       08 d0 4d e2     sub     sp, sp, #8
>   ; bar();
>          c:       fe ff ff eb     bl      #-8 <foo+0xc>
>   ; int i = 2;
>         10:       02 e0 a0 e3     mov     lr, #2
>         14:       04 e0 8d e5     str     lr, [sp, #4]
>   ; return i;
>         18:       04 00 9d e5     ldr     r0, [sp, #4]
>         1c:       0b d0 a0 e1     mov     sp, r11
>         20:       00 88 bd e8     pop     {r11, pc}
>
>
> It is not clear to me whether this is an expected change but I thought I would let you know about it if it actually isn't.
>
> Thanks,
> Petr


Hi @petpav01, the behavior difference you're seeing is an expected result of this patch. The tradeoff is to give up some debug info precision on single-use constant materialization in order to improve single-stepping when constants are reused. I'm happy to try and find a better solution if this regresses some behavior you depend on.


Repository:
  rL LLVM

https://reviews.llvm.org/D48468