search

Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

[Debuginfo] @llvm.dbg.value changes behaviour of "Speculatively execute instructions" #39544

Open Quuxplusone opened 5 years ago

Quuxplusone commented 5 years ago
Bugzilla Link PR40573
Status NEW
Importance P enhancement
Reported by Jonas Paulsson (paulsson@linux.vnet.ibm.com)
Reported on 2019-02-02 09:29:42 -0800
Last modified on 2019-02-03 13:35:01 -0800
Version trunk
Hardware PC Linux
CC dblaikie@gmail.com, ditaliano@apple.com, llvm-bugs@lists.llvm.org, paulsson@linux.vnet.ibm.com, paul_robinson@playstation.sony.com, uweigand@de.ibm.com
Fixed by commit(s)
Attachments tc_speculexe.ll (123182 bytes, text/plain)
Blocks
Blocked by
See also 
Created attachment 21421
reduced testcase

Without the @llvm.dbg instruction in the test case, the fdiv instruction is
moved up in the CFG by

bin/opt -mtriple=systemz-unknown -mcpu=z13 -S tc_speculexe.ll -speculative-
execution

I suspect that this is also (as in bug 40564) related to counting instructions
in the block on the line

     if (NotHoisted.size() > SpecExecMaxNotHoisted)

in SpeculativeExecution.cpp
Quuxplusone commented 5 years ago

Attached tc_speculexe.ll (123182 bytes, text/plain): reduced testcase

Quuxplusone commented 5 years ago 
So, just to understand, this is the thing not hoisted.

bb3:                                              ; preds = %bb
  %tmp4 = fdiv double 1.000000e+00, %tmp, !dbg !1200
Quuxplusone commented 5 years ago 
(In reply to Davide Italiano from comment #1)
> So, just to understand, this is the thing not hoisted.
>
> bb3:                                              ; preds = %bb
>   %tmp4 = fdiv double 1.000000e+00, %tmp, !dbg !1200

yes, exactly...
Quuxplusone commented 5 years ago 
This is why it's failing.

   200      // If a block has only one instruction, then that is a terminator
   201      // instruction so that the block does nothing. This does happen.
   202      if (Succ1.size() == 1) // equivalent to if-then
-> 203        return considerHoistingFromTo(Succ0, B);
Quuxplusone commented 5 years ago 
bb5:                                              ; preds = %bb
  br label %bb6
Quuxplusone commented 5 years ago 
OK, I think I know how to fix this, I'll submit a patch soon.
Any chance you can reduce this a bit? The testcase is 2000 lines, but I guess
only a small subset of them is needed.
Quuxplusone commented 5 years ago 
(In reply to Davide Italiano from comment #5)
> OK, I think I know how to fix this, I'll submit a patch soon.
> Any chance you can reduce this a bit? The testcase is 2000 lines, but I
> guess only a small subset of them is needed.

I can only suggest that you try

bin/opt -strip -metarenamer -debugify

...

I did reduce it from the benchmark, but I may have missed using -debugify as
well. Just need to make sure to not loose the llvm.dbg instruction...