[PATCH] D18802: Improve support for i386 and i486 CPUs.

[James Y Knight via llvm-commits]

jyknight added a comment.

> For BSWAP: would it work to do the custom lowering at ISel time instead of using a pseudo? I think it's possible to even write a pattern.


I don't know, is it? You'd need to be able to express the operation "rotate bottom 16 bits of this register by 8 bits, in place, leaving the upper 16 bits as they were". It's not a normal 16-bit operation, which would let you allocate 16-bit inputs/output virtual registers, because the contents of the upper bits are important.


================
Comment at: lib/Target/X86/X86.td:279
@@ -272,1 +278,3 @@
+def : Proc<"pentium-mmx",     [FeatureX87, FeatureSlowUAMem16, FeatureMMX, Feature586Insns]>;
+def : Proc<"i686",            [FeatureX87, FeatureSlowUAMem16, Feature586Insns]>;
 def : Proc<"pentiumpro",      [FeatureX87, FeatureSlowUAMem16, FeatureCMOV]>;
----------------
ab wrote:
> Should these features be on the remaining models?  It'd probably be time to do ProcessorFeatures lists like SNB.
I think it doesn't need to be, because I added it as implied by cmov, which everything else supports?

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:96
@@ -96,1 +95,3 @@
+    else if (Subtarget.has486Insns())
+      setMaxAtomicSizeInBitsSupported(32); // has cmpxchg
   }
----------------
RKSimon wrote:
> Subtarget.has486Insns() and Subtarget.has586Insns() are quiet vague about actual features, would it not be clearer to instead add Subtarget.hasCMPXCHG8() and Subtarget.hasCMPXCHG() that then uses Has586/Has486 internally?
> 
> Same for BSWAP (Subtarget.hasBSWAP()) below.
Sure, sounds fine.


http://reviews.llvm.org/D18802