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

[Eric Christopher via llvm-commits]

echristo added a subscriber: echristo.

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:96
@@ -96,1 +95,3 @@
+    else if (Subtarget.has486Insns())
+      setMaxAtomicSizeInBitsSupported(32); // has cmpxchg
   }
----------------
jyknight wrote:
> 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.
Couple of comments here:

a) I definitely like the choice of better function naming for whether or not we have a particular cpu feature.
b) I'm not sure whether or not we want to conditionalize the 486/586 insns on sets of subtarget features or a broad ISA level feature. Unlike most of the later instructions added there's very little "subsetting" that we can do for this, that said, it forms a contrast with the rest of the port where we do have subtarget features for everything not baseline pentium.
c) That leads us to "hey, why not just set pentium as the baseline set of features and stop letting people select 386/486 as part of their compiles". Any thoughts here? 


http://reviews.llvm.org/D18802