Page MenuHomePhabricator

[SystemZ] Add a subtarget cache like some other targets already have.
ClosedPublic

Authored by jonpa on Feb 5 2020, 1:37 PM.

Details

Summary

Each function is with this compiled with the SystemZSubtarget initialized from the functions attributes.

We need the "+soft-float" feature in UsesVectorABI(), so we have the front end add it, so we don't need to check "-use-soft-float"="true", like the other targets, right?

Diff Detail

Event Timeline

jonpa created this revision.Feb 5 2020, 1:37 PM
Herald added a project: Restricted Project. · View Herald Transcript
Herald added a subscriber: hiraditya. · View Herald Transcript

We need the "+soft-float" feature in UsesVectorABI(), so we have the front end add it, so we don't need to check "-use-soft-float"="true", like the other targets, right?

Yes, I think that's correct.

llvm/test/CodeGen/SystemZ/function-attributes-01.ll
12

Would be good to have some tests with "target-features"="-soft-float" as well.

31

This seems strange, an explicit per-function +soft-float should override the general setting, right?

64

Same here.

jonpa updated this revision to Diff 243287.Feb 7 2020, 2:34 PM
jonpa marked 5 inline comments as done.

Patch updated.

It seems that the general precedence (when invoking llc) is that first (in reverse order) is "target-features", and then -mattr. So for example a function with the attribute "target-features"="-vector" compiled with llc -mattr=+vector, gets a (SystemZTargetMachine::getSubtargetImpl) FS -vector,+vector, meaning that +vector wins.

It's interesting that the function attribute "target-cpu" seems to override the llc option -mcpu, while llc option -mattr= seems to override function attributes, like +-vector.

To get the behaviour you expected for soft-float (like also other targets seem to have), it seems easiest then to do like the other targets and check for the "use-soft-float"="true" attribute, and in that case add a final and decisive +soft-float to FS (this is not the way e.g. [+-]vector works per above, but maybe there is a reason for this?).

A function with "use-soft-float"="true" now always gets soft-float, while "use-soft-float"="false" will take no effect, so that -mattr=soft-float actually will give soft-float.

llvm/test/CodeGen/SystemZ/function-attributes-01.ll
12

I updated the tests to have "use-soft-float"="false" or "true", which is what all functions get from the clang front end.

uweigand accepted this revision.Feb 10 2020, 8:01 AM

It seems that the general precedence (when invoking llc) is that first (in reverse order) is "target-features", and then -mattr. So for example a function with the attribute "target-features"="-vector" compiled with llc -mattr=+vector, gets a (SystemZTargetMachine::getSubtargetImpl) FS -vector,+vector, meaning that +vector wins.

Huh, that seems unexpected. But as long as it's not a SystemZ problem, I guess it's just the way it is ...

To get the behaviour you expected for soft-float (like also other targets seem to have), it seems easiest then to do like the other targets and check for the "use-soft-float"="true" attribute, and in that case add a final and decisive +soft-float to FS (this is not the way e.g. [+-]vector works per above, but maybe there is a reason for this?).

OK, let's just mirror what everybody else is doing, then.

Patch LGTM. Thanks!

This revision is now accepted and ready to land.Feb 10 2020, 8:01 AM
This revision was automatically updated to reflect the committed changes.