pengfei accepted this revision. pengfei added a comment. This revision is now accepted and ready to land.
Then I'm OK with this change. ================ Comment at: llvm/lib/Support/X86TargetParser.cpp:531 constexpr FeatureBitset ImpliedFeaturesSSSE3 = FeatureSSE3; constexpr FeatureBitset ImpliedFeaturesSSE4_1 = FeatureSSSE3; constexpr FeatureBitset ImpliedFeaturesSSE4_2 = FeatureSSE4_1; ---------------- craig.topper wrote: > pengfei wrote: > > tianqing wrote: > > > pengfei wrote: > > > > hjl.tools wrote: > > > > > pengfei wrote: > > > > > > Can we let `ImpliedFeaturesSSE4_1 = FeatureSSSE3 | FeaturesCRC32` > > > > > > so that we don't need to add `crc32` on sse4.1 and above? > > > > > SSE4.1 implies CRC32. But CRC32 shouldn't imply SSE4.1. > > > > Yes. The constexpr here means `FeaturesSSE4_1` implies both > > > > `FeatureSSSE3` and `FeaturesCRC32`. > > > CRC32 was added in SSE4.2. > > > > > > In LLVM this implication relationship is bidirectional, that is: > > > > > > * -msse4.2 implies -mcrc32 > > > * -mcrc32 doesn't implies -msse4.2. > > > * -mno-sse4.2 doesn't implies -mno-crc32. > > > * But -mno-crc32 also implies -mno-sse4.2. > > Sorry, I mistook SSE4.1 with SSE4.2. I meant to `constexpr FeatureBitset > > ImpliedFeaturesSSE4_2 = FeatureSSE4_1 | FeaturesCRC32;` then. > > I see you make "-msse4.2 implies -mcrc32" by FE. Changing here should make > > it implies in backend, so that you don't need to explicitly add crc32 in > > LLVM tests. > This file is only used by the frontend and it creates a bidirectional > relationship. It would make -msse4.2 imply -mcrc32. But it also makes > -mno-crc32 imply -mno-sse4.2. Just like -mno-sse4.1 implies -mno-sse4.2. But > that's not what we want. Got it. Thanks for the explanation. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105462/new/ https://reviews.llvm.org/D105462 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits