Page MenuHomePhabricator

Ignore dbg intrinsics in loop-reroll
ClosedPublic

Authored by weimingz on Sep 24 2015, 4:29 PM.

Details

Summary

Currently, we get different code with and without -g. This patch marks debug inst and a few other LLVM intrinsics as ignorable for loop-reroll

Diff Detail

Repository
rL LLVM

Event Timeline

weimingz updated this revision to Diff 35686.Sep 24 2015, 4:29 PM
weimingz retitled this revision from to Ignore dbg intrinsics in loop-reroll.
weimingz updated this object.
weimingz set the repository for this revision to rL LLVM.
weimingz added a subscriber: llvm-commits.
vsk added a subscriber: vsk.Sep 24 2015, 6:51 PM

Is it possible to add a test for this? E.g something that would force the debug message to be printed without your patch.

weimingz updated this revision to Diff 35751.Sep 25 2015, 11:55 AM
weimingz removed rL LLVM as the repository for this revision.

added test case

In D13150#253180, @vsk wrote:

Is it possible to add a test for this? E.g something that would force the debug message to be printed without your patch.

Sure. Please review patch set 2.

hfinkel edited edge metadata.Sep 25 2015, 4:21 PM
lib/Transforms/Scalar/LoopRerollPass.cpp
1007 ↗(On Diff #35751)

Why these and not others? I suspect that we could get:

case Intrinsic::annotation:
case Intrinsic::ptr_annotation:
case Intrinsic::var_annotation:

and also:

case Intrinsic::invariant_start:
case Intrinsic::invariant_end:
case Intrinsic::lifetime_start:
case Intrinsic::lifetime_end:

but for these, at least, we'll need test cases because getting them wrong will miscompile code, and also, they're normally associated with bitcasts to get the pointer type right and you'd need to deal with those too.

weimingz updated this revision to Diff 35784.Sep 25 2015, 5:10 PM
weimingz edited edge metadata.

adds ptr_annotation, val_annotation. Not sure if we should add lifetime_start/end, invariant_start/end, so I put them under TODO

weimingz added inline comments.Sep 25 2015, 5:13 PM
lib/Transforms/Scalar/LoopRerollPass.cpp
1007 ↗(On Diff #35784)

I added those annotations into white list. I'm not very sure about the rest. I will try to see if I can get some test case

hfinkel accepted this revision.Sep 25 2015, 5:50 PM
hfinkel edited edge metadata.

LGTM.

This revision is now accepted and ready to land.Sep 25 2015, 5:50 PM
This revision was automatically updated to reflect the committed changes.