Page MenuHomePhabricator

[Attributor] Introduce a helper do deal with constant type mismatches
ClosedPublic

Authored by jdoerfert on Mon, Jun 7, 4:53 PM.

Details

Summary

If we simplify values we sometimes end up with type mismatches. If the
value is a constant we can often cast it though to still allow
propagation. The logic is now put into a helper and it replaces some
ad hoc things we did before.

This also introduces the AA namespace for abstract attribute related
functions and types.

Diff Detail

Event Timeline

jdoerfert created this revision.Mon, Jun 7, 4:53 PM
jdoerfert requested review of this revision.Mon, Jun 7, 4:53 PM
Herald added a project: Restricted Project. · View Herald TranscriptMon, Jun 7, 4:53 PM
Herald added a subscriber: bbn. · View Herald Transcript
kuter added inline comments.Thu, Jun 10, 3:04 PM
llvm/lib/Transforms/IPO/AttributorAttributes.cpp
4637–4638

NIT: &V != C is already checked and must be true.

4779–4780

I think the associated value for a RETURNED position is the value to the function itself so I don't think it makes sense to do this check.

jdoerfert added inline comments.Thu, Jun 10, 3:25 PM
llvm/lib/Transforms/IPO/AttributorAttributes.cpp
4637–4638

Good point, I wanted NewV != C. Will be fixed.

4779–4780

You are right. I'll remove the V checks here.

ormris removed a subscriber: ormris.Fri, Jun 11, 2:52 PM
jdoerfert updated this revision to Diff 352603.Wed, Jun 16, 7:11 PM
jdoerfert marked 2 inline comments as done.

Address review

jdoerfert added inline comments.Thu, Jun 17, 6:43 AM
llvm/lib/Transforms/IPO/AttributorAttributes.cpp
4779–4780

The problem is that we default to the original value in SimplifiedAssociatedValue which doesn't really work for this position but is harder to fix.

uenoku added inline comments.Thu, Jun 17, 7:35 AM
llvm/lib/Transforms/IPO/Attributor.cpp
171

Probably poison too

jdoerfert added inline comments.Thu, Jun 17, 9:00 AM
llvm/lib/Transforms/IPO/Attributor.cpp
171

PoisonValue inherits from UndefValue, as poison can degenerate to undef. That said, I'll make it explicit to keep it as poison if it was (for the future),

Rebase + address comment

jdoerfert marked an inline comment as done.Thu, Jun 17, 11:19 AM
kuter accepted this revision.Thu, Jun 17, 11:53 AM

LGTM

This revision is now accepted and ready to land.Thu, Jun 17, 11:53 AM
This revision was landed with ongoing or failed builds.Thu, Jun 17, 11:11 PM
This revision was automatically updated to reflect the committed changes.