[PATCH] D105462: [X86] Add CRC32 feature.

[Pengfei Wang via Phabricator via cfe-commits]

pengfei added inline comments.


================
Comment at: llvm/lib/Support/X86TargetParser.cpp:531
 constexpr FeatureBitset ImpliedFeaturesSSSE3 = FeatureSSE3;
 constexpr FeatureBitset ImpliedFeaturesSSE4_1 = FeatureSSSE3;
 constexpr FeatureBitset ImpliedFeaturesSSE4_2 = FeatureSSE4_1;
----------------
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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105462/new/

https://reviews.llvm.org/D105462