[PATCH] Add HLE target feature

[Nadav Rotem]

Michael, 

I appreciate you rebasing this patch, but this is not helping at all.  I asked you specific questions and you ignored them. All of this code that you are adding has a cost. Its making the compiler more complex, and it will make it difficult for us to refactor major parts of the compiler. Moreover, it increases the compile time. Every time that you add special handling for some feature you care about you increase the size of the compiler. Every feature that you add adds dozens of IFs and hooks that need to be executed by innocent people who don't care about this. I asked you a simple question. Why does HLE support need to be a part of LLVM ? HLE instructions are probably awesome, but they do not belong in the compiler. I could be wrong, but it is your responsibility to convince us why we should pay the compile time and complexity costs. 

Thanks,
Nadav 


On Feb 27, 2013, at 1:37 PM, darkbuck <michael.hliao at gmail.com> wrote:

>  the patch is rebased to trunk and latest changes on replacing TargetFlags to HLEHint
> 
> Hi nadav, jfifield, hfinkel, chandlerc, eli.friedman,
> 
> http://llvm-reviews.chandlerc.com/D437
> 
> CHANGE SINCE LAST DIFF
>  http://llvm-reviews.chandlerc.com/D437?vs=1053&id=1142#toc
> 
> Files:
>  lib/Target/X86/X86.td
>  lib/Target/X86/X86InstrInfo.td
>  lib/Target/X86/X86Subtarget.cpp
>  lib/Target/X86/X86Subtarget.h
> 
> Index: lib/Target/X86/X86.td
> ===================================================================
> --- lib/Target/X86/X86.td
> +++ lib/Target/X86/X86.td
> @@ -120,6 +120,8 @@
>                                       "Support BMI2 instructions">;
> def FeatureRTM     : SubtargetFeature<"rtm", "HasRTM", "true",
>                                       "Support RTM instructions">;
> +def FeatureHLE     : SubtargetFeature<"hle", "HasHLE", "true",
> +                                      "Support HLE">;
> def FeatureADX     : SubtargetFeature<"adx", "HasADX", "true",
>                                       "Support ADX instructions">;
> def FeatureLeaForSP : SubtargetFeature<"lea-sp", "UseLeaForSP", "true",
> @@ -201,7 +203,7 @@
>                                FeatureRDRAND, FeatureF16C, FeatureFSGSBase,
>                                FeatureMOVBE, FeatureLZCNT, FeatureBMI,
>                                FeatureBMI2, FeatureFMA,
> -                               FeatureRTM]>;
> +                               FeatureRTM, FeatureHLE]>;
> 
> def : Proc<"k6",              [FeatureMMX]>;
> def : Proc<"k6-2",            [Feature3DNow]>;
> Index: lib/Target/X86/X86InstrInfo.td
> ===================================================================
> --- lib/Target/X86/X86InstrInfo.td
> +++ lib/Target/X86/X86InstrInfo.td
> @@ -603,6 +603,7 @@
> def HasBMI       : Predicate<"Subtarget->hasBMI()">;
> def HasBMI2      : Predicate<"Subtarget->hasBMI2()">;
> def HasRTM       : Predicate<"Subtarget->hasRTM()">;
> +def HasHLE       : Predicate<"Subtarget->hasHLE()">;
> def HasADX       : Predicate<"Subtarget->hasADX()">;
> def FPStackf32   : Predicate<"!Subtarget->hasSSE1()">;
> def FPStackf64   : Predicate<"!Subtarget->hasSSE2()">;
> Index: lib/Target/X86/X86Subtarget.cpp
> ===================================================================
> --- lib/Target/X86/X86Subtarget.cpp
> +++ lib/Target/X86/X86Subtarget.cpp
> @@ -310,6 +310,10 @@
>         HasBMI = true;
>         ToggleFeature(X86::FeatureBMI);
>       }
> +      if ((EBX >> 4) & 0x1) {
> +        HasHLE = true;
> +        ToggleFeature(X86::FeatureHLE);
> +      }
>       if (IsIntel && ((EBX >> 5) & 0x1)) {
>         X86SSELevel = AVX2;
>         ToggleFeature(X86::FeatureAVX2);
> @@ -439,6 +443,7 @@
>   HasBMI = false;
>   HasBMI2 = false;
>   HasRTM = false;
> +  HasHLE = false;
>   HasADX = false;
>   IsBTMemSlow = false;
>   IsUAMemFast = false;
> Index: lib/Target/X86/X86Subtarget.h
> ===================================================================
> --- lib/Target/X86/X86Subtarget.h
> +++ lib/Target/X86/X86Subtarget.h
> @@ -121,6 +121,9 @@
>   /// HasRTM - Processor has RTM instructions.
>   bool HasRTM;
> 
> +  /// HasHLE - Processor has HLE.
> +  bool HasHLE;
> +
>   /// HasADX - Processor has ADX instructions.
>   bool HasADX;
> 
> @@ -253,6 +256,7 @@
>   bool hasBMI() const { return HasBMI; }
>   bool hasBMI2() const { return HasBMI2; }
>   bool hasRTM() const { return HasRTM; }
> +  bool hasHLE() const { return HasHLE; }
>   bool hasADX() const { return HasADX; }
>   bool isBTMemSlow() const { return IsBTMemSlow; }
>   bool isUnalignedMemAccessFast() const { return IsUAMemFast; }
> <D437.2.patch>_______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130227/d3873c88/attachment.html>