Page MenuHomePhabricator

[InstCombine] eliminate a pointer cast around insertelement
ClosedPublic

Authored by spatel on Aug 10 2020, 6:32 AM.

Details

Summary

I'm not sure if this solves PR46839 completely, but reducing the casting should help:
https://bugs.llvm.org/show_bug.cgi?id=46839

I have not dealt much with pointer casts, so if anyone sees any corner-cases to test/avoid, please comment. Alive2 does not appear to support these ops.

Diff Detail

Event Timeline

spatel created this revision.Aug 10 2020, 6:32 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 10 2020, 6:32 AM
spatel requested review of this revision.Aug 10 2020, 6:32 AM
nikic accepted this revision.Aug 10 2020, 11:56 AM

Looks good to me. You already test the zext/trunc cases. I think this fold is also not problematic wrt https://bugs.llvm.org/show_bug.cgi?id=34548.

This revision is now accepted and ready to land.Aug 10 2020, 11:56 AM

If we're eliminating an inttoptr/ptrtoint pair, do we need to check that the pointer type is large enough to hold the integer type? (I don't know how likely this is to come up given instcombine canonicalizes the casts, but better to be safe, I think.)

I agree this transformation is safe modulo the pointer size check. It should be fine to fold p2i(i2p i) -> i when the size of pointer is large enough.

spatel updated this revision to Diff 284885.Aug 11 2020, 1:59 PM

Patch updated:
Given the transform above this one and the type checks included there and here, I think we can assert that sizes match. Let me know if that handles the possible problem patterns that we must avoid.

efriedma accepted this revision.Aug 11 2020, 2:05 PM

Oh, I see, the transform to canonicalize the integer width always runs before. LGTM

LGTM, thank you!