Page MenuHomePhabricator

[CostModel] Use isExtLoad in BasicTTI
Needs ReviewPublic

Authored by samparker on Mon, Apr 27, 8:57 AM.



Compared against D78922, use isExtLoad instead of isLoadExtLegal which causes some changes around vectorization for X86.

Diff Detail

Event Timeline

samparker created this revision.Mon, Apr 27, 8:57 AM
Herald added a project: Restricted Project. · View Herald TranscriptMon, Apr 27, 8:57 AM
RKSimon added inline comments.Thu, May 7, 11:11 AM

Maybe keep this separate from the int extends?

samparker updated this revision to Diff 265455.Thu, May 21, 1:29 AM
samparker added reviewers: Ayal, jsji, hfinkel, gilr.
samparker added a subscriber: dmgreen.

Now rebased against master. This is making some (surprising to me) changes to vector code which I can only assume is due to the concerns which @dmgreen raised in D78922. I tried a heavy-handed approach of not using TLI calls for vector casts but that didn't look like a suitable option. So it would be really good to hear from the X86 and PPC guys on whether the vector code changes look sane or whether TLI is just causing more confusion.

This would conflict with D79162. That's not the prettiest patch in the world, but it solves some real problems we have in the vectorizer and moving further away from it seems like a mistake.

I get that given an (accurate) context instruction I, it's good to make use of it to produce better costs. But ideally the costmodel should always be costing hypothetical instructions, not relying on real ones.

On a concrete level, this might now be using the type of the actually load (say i32), and not the type the costmodel has provided (v4i32, for example)?

ideally the costmodel should always be costing hypothetical instructions, not relying on real ones.

This probably isn't true for most optimisations though, but of course the vectorizer is an important (but small) user of this. I would suspect that vectorizer should only really be passing the instruction when calculating the scalar cost of the loop. But I'll revisit this once the context stuff is in.