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

[A. Skrobov via llvm-commits]

tyomitch added a comment.

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.


http://reviews.llvm.org/D15746