[PATCH] D15746: Normalize the features string in SubtargetFeatures::getFeatureBits

[Eric Christopher via llvm-commits]

echristo added a comment.

In http://reviews.llvm.org/D15746#342099, @tyomitch wrote:

> Thank you Eric for your detailed comments!
>
> Do I understand correctly that this is your main concern:
>
> > OK, that almost makes sense, however:
>
> > 
>
> > As a side, but important, note: I was going to use -mno-sse3 -msse3 as part of this example. It appears that the front end currently will take that as a last one wins and ignore the -mno-sse3. What would happen in your testcase if it were -, -, +? Last one wins?
>
> > 
>
> > Do we want last one wins here? Do we want all pairs to be merged and the one that's not a pair gone? Before we handle earlier ones? Ultimately I think we might want to assume that canonicalization has happened before we get to this point, i.e. maybe even disallow attribute strings that turn off features that were explicitly turned on earlier (not implicitly mind).
>
>
> That's correct, my patch keeps the existing "last one wins" handling: "+sse3,+sse3,-sse3" would (still) be treated identically to "-sse3".
>  The only case where "last one wins" doesn't currently apply, is when a super-feature is disabled by disabling a sub-feature, but isn't reenabled afterwards.
>  In other words, "+fullfp16,-fp-armv8,+fp-armv8" isn't currently equivalent to "+fullfp16,+fp-armv8".
>  My patch would fix this inconsistency, so that "last one wins" would apply universally.


I'm still uncertain we want to do this in the backend. I seem to recall you wanting to do this for a reason, but searching my email can't find it. Why can't we rely/assert/verify that the list of features in the list is complete and non-contradictory rather than coping with "garbage" in the backend. This seems like something the front-end, or whatever is generating IR, should handle.


http://reviews.llvm.org/D15746