[PATCH] D28742: [InstCombine] Don't DSE across readnone functions that may throw

[Sanjoy Das via llvm-commits]

Hi,

On Sun, Jan 15, 2017 at 11:46 AM, Daniel Berlin <dberlin at dberlin.org> wrote:
> FWIW: I agree.
> But i'd go farther.
> I think at this point, ->hasSideEffects (or ->sideEffects) may just want to
> return a mask, so that the above just becomes
>
> ->sideEffects & (MayWrite | MayRead | MayThrow)
>
> or whatever nice version we want.

Yes, that's a good idea.  I suspect we'll still want to keep things
like mayRead() and mayThrow(), since it would be tedious to have to
write

 -> sideEffects() & Instruction::MayThrow

when we care about just one single thing, but I like the idea.

I'm not sure about the name though -- generally, most people would not
say MayRead is a side-effect.  I was thinking of something like
"implicitBehavior()", since this really is an enum of things that an
instruction can do that are not explicitly specified in SSA / CFG.

One thing though: let's not stall this set of patches on solving that
other problem. :)
-- Sanjoy

>
> I think we've added enough accessors at this point, all recursively defined,
> that i think it's becoming a bit of a mess ;)
>
> But others may disagree :)
>
> On Sun, Jan 15, 2017 at 11:41 AM, Sanjoy Das via Phabricator
> <reviews at reviews.llvm.org> wrote:
>>
>> sanjoy added a comment.
>>
>> In https://reviews.llvm.org/D28742#646705, @majnemer wrote:
>>
>> > Instead of using `I->mayReadFromMemoryOrThrow() ||
>> > I->mayWriteToMemory()`, I'd recommend using `I->mayReadFromMemory() ||
>> > I->mayHaveSideEffects()`
>>
>>
>> What do you think about `I->mayReadFromMemory() || I->mayWriteToMemory()
>> || I->mayThrow()`?  I don't particularly like that we club may-throw with
>> has-side-effects.
>>
>>
>> https://reviews.llvm.org/D28742
>>
>>
>>
>