Page MenuHomePhabricator

[InstCombine] Don't blow up in foldICmpWithCastAndCast on vector icmp instructions.

Authored by dneilson on Mar 2 2018, 6:01 PM.



Presently, InstCombiner::foldICmpWithCastAndCast() implicitly assumes that it is
only invoked with icmp instructions of integer type. If that assumption is broken,
and it is called with an icmp of vector type, then it fails (asserts/crashes).

This patch addresses the deficiency. It allows it to simplify
icmp (ptrtoint x), (ptrtoint/c) of vector type into a compare of the inputs,
much as is done when the type is integer.

Diff Detail


Event Timeline

dneilson created this revision.Mar 2 2018, 6:01 PM
anna added inline comments.Mar 5 2018, 8:03 AM
3411 ↗(On Diff #136890)

Instead of capturing everything, let's capture just DL.

dneilson added inline comments.Mar 5 2018, 8:26 AM
3411 ↗(On Diff #136890)

This is not capturing everything, it is only capturing the this pointer.


[&] captures all automatic variables **used in the body** of the lambda by reference and current object by reference if exists (emphasis mine).

There are no automatics used in the body of this lambda, so only the current object reference (i.e. the this pointer) is captured.

I could change [&] to [this] to make it more explicit, if desired...

anna added inline comments.Mar 5 2018, 9:05 AM
3411 ↗(On Diff #136890)

But why do you need to capture this? Basically, this should do right:

const auto& CompatibleSizes = [&DL](Type* SrcTy, Type* DestTy) -> bool {

It makes it clearer that the only capture required is DL which is used in the lambda.

dneilson added inline comments.Mar 5 2018, 9:08 AM
3411 ↗(On Diff #136890)

Non-obvious detail: DL is a member of the InstCombiner class, and not an automatic.

You can't capture members. Can only capture reference to current object (i.e. this), and automatics.

anna accepted this revision.Mar 5 2018, 9:27 AM


3411 ↗(On Diff #136890)

ah I didn't see that.

This revision is now accepted and ready to land.Mar 5 2018, 9:27 AM
This revision was automatically updated to reflect the committed changes.