This is an archive of the discontinued LLVM Phabricator instance.

Teach poison value tracking that certain calls always terminate

Authored by sanjoy on Apr 17 2016, 8:50 PM.



This makes our poison value analysis a little smarter.

Diff Detail

Event Timeline

sanjoy updated this revision to Diff 54026.Apr 17 2016, 8:50 PM
sanjoy retitled this revision from to Teach poison value tracking that certain calls always terminate.
sanjoy updated this object.
sanjoy added reviewers: bjarke.roune, broune.
sanjoy added a subscriber: llvm-commits.
majnemer added inline comments.

Couldn't this just be !I->mayHaveSideEffects() ?

sanjoy added inline comments.Apr 18 2016, 12:13 AM

Yeah, but I didn't want to write it that way -- IMO it is only "incidental" that mayHaveSideEffects and what I'm computing here are the same implementation. IOW, the predicate in the two cases (mayHaveSideEffects vs. "guaranteed to transfer control flow to successor") are quite different, even if they have the same implementation.

broune accepted this revision.Apr 21 2016, 4:24 PM
broune edited edge metadata.
broune added inline comments.

This seems unfortunate given that, as I understand it, we then can't compile C11 correctly:

Go ahead and submit this if you're sure that the consensus is that this is how LLVM ought to behave. I'd be more comfortable if the langref stated that infinite side-effect free loops are not allowed, but I couldn't find anything about that in there.

This revision is now accepted and ready to land.Apr 21 2016, 4:24 PM
sanjoy added inline comments.Apr 21 2016, 4:31 PM

I'm going to look at , but assuming that's accurate (i.e. while (1); is well defined) I'll redact this patch. It's not really an option (IMO) to miscompile C11. :)

hfinkel added inline comments.

As I understand it, LLVM is not really self-consistent on this point. As you point out, we we will DCE readnone and readonly calls. We should be, however, because we have frontends for languages with well-defined infinite loops (Java?, and apparently C11), and those without well-defined infinite loops (C++), and we'd like to do the best job on all of them. I think we need a new function-level attribute that controls whether we get to assume that loops terminate, and then we can make a consistent decision everywhere.

sanjoy abandoned this revision.May 30 2016, 1:31 PM

The "icmp propagages poison" bit has been checked in in rL271150. The "branch on poison is UB" bit is incorrect, and has been abandoned.