<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">Michael, <div><br></div><div>Thanks for working on this. I agree that LLVM already supports atomic operations, but I don't understand why you want to add transactional memory into the compiler. This clearly belongs in a library, not in the compiler. What you are saying about the implementation may be correct, but it is irrelevant to the discussion about the HLE. I don't think that HLE belongs in the compiler. Why do you think that it belongs to the compiler and not to a library ? </div><div><br></div><div>Thanks,</div><div>Nadav</div><div><br></div><div><br><div><div>On Feb 27, 2013, at 3:09 PM, Michael Liao <<a href="mailto:michael.liao@intel.com">michael.liao@intel.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;">Hi Nadav<br><br>"LLVM Atomic Instructions and Concurrency Guide" puts more details on<br>why atomic instructions are added in LLVM IR and optimizations around<br>it. For HLE support, you could treat an enhancement on atomic<br>instruction by taking advantage of hardware support or TM support, the<br>current approach is to add hint in atomic instructions already existed<br>in LLVM IR to help backend generate proper code. By taking this<br>approach, as compiler developers, we leverage the existing<br>infrastructure/optimization as much as possible; as application<br>developers, they have the minimal change to adapt HLE technology into<br>their code and keep code across different compilers.<br><br>Yours<br>- Michael<br><br>On Wed, 2013-02-27 at 14:08 -0800, Nadav Rotem wrote:<br><blockquote type="cite">Michael,<span class="Apple-converted-space"> </span><br><br><br>I appreciate you rebasing this patch, but this is not helping at all.<br>I asked you specific questions and you ignored them. All of this code<br>that you are adding has a cost. Its making the compiler more complex,<br>and it will make it difficult for us to refactor major parts of the<br>compiler. Moreover, it increases the compile time. Every time that you<br>add special handling for some feature you care about you increase the<br>size of the compiler. Every feature that you add adds dozens of IFs<br>and hooks that need to be executed by innocent people who don't care<br>about this. I asked you a simple question. Why does HLE support need<br>to be a part of LLVM ? HLE instructions are probably awesome, but they<br>do not belong in the compiler. I could be wrong, but it is your<br>responsibility to convince us why we should pay the compile time and<br>complexity costs.<span class="Apple-converted-space"> </span><br><br><br>Thanks,<br>Nadav<span class="Apple-converted-space"> </span><br><br><br><br>On Feb 27, 2013, at 1:37 PM, darkbuck <<a href="mailto:michael.hliao@gmail.com">michael.hliao@gmail.com</a>> wrote:<br><br><blockquote type="cite">the patch is rebased to trunk and latest changes on replacing<br>TargetFlags to HLEHint<br><br>Hi nadav, jfifield, hfinkel, chandlerc, eli.friedman,<br><br><a href="http://llvm-reviews.chandlerc.com/D437">http://llvm-reviews.chandlerc.com/D437</a><br><br>CHANGE SINCE LAST DIFF<br>http://llvm-reviews.chandlerc.com/D437?vs=1053&id=1142#toc<br><br>Files:<br>lib/Target/X86/X86.td<br>lib/Target/X86/X86InstrInfo.td<br>lib/Target/X86/X86Subtarget.cpp<br>lib/Target/X86/X86Subtarget.h<br><br>Index: lib/Target/X86/X86.td<br>===================================================================<br>--- lib/Target/X86/X86.td<br>+++ lib/Target/X86/X86.td<br>@@ -120,6 +120,8 @@<br> "Support BMI2 instructions">;<br>def FeatureRTM : SubtargetFeature<"rtm", "HasRTM", "true",<br> "Support RTM instructions">;<br>+def FeatureHLE : SubtargetFeature<"hle", "HasHLE", "true",<br>+ "Support HLE">;<br>def FeatureADX : SubtargetFeature<"adx", "HasADX", "true",<br> "Support ADX instructions">;<br>def FeatureLeaForSP : SubtargetFeature<"lea-sp", "UseLeaForSP",<br>"true",<br>@@ -201,7 +203,7 @@<br> FeatureRDRAND, FeatureF16C,<br>FeatureFSGSBase,<br> FeatureMOVBE, FeatureLZCNT,<br>FeatureBMI,<br> FeatureBMI2, FeatureFMA,<br>- FeatureRTM]>;<br>+ FeatureRTM, FeatureHLE]>;<br><br>def : Proc<"k6", [FeatureMMX]>;<br>def : Proc<"k6-2", [Feature3DNow]>;<br>Index: lib/Target/X86/X86InstrInfo.td<br>===================================================================<br>--- lib/Target/X86/X86InstrInfo.td<br>+++ lib/Target/X86/X86InstrInfo.td<br>@@ -603,6 +603,7 @@<br>def HasBMI : Predicate<"Subtarget->hasBMI()">;<br>def HasBMI2 : Predicate<"Subtarget->hasBMI2()">;<br>def HasRTM : Predicate<"Subtarget->hasRTM()">;<br>+def HasHLE : Predicate<"Subtarget->hasHLE()">;<br>def HasADX : Predicate<"Subtarget->hasADX()">;<br>def FPStackf32 : Predicate<"!Subtarget->hasSSE1()">;<br>def FPStackf64 : Predicate<"!Subtarget->hasSSE2()">;<br>Index: lib/Target/X86/X86Subtarget.cpp<br>===================================================================<br>--- lib/Target/X86/X86Subtarget.cpp<br>+++ lib/Target/X86/X86Subtarget.cpp<br>@@ -310,6 +310,10 @@<br> HasBMI = true;<br> ToggleFeature(X86::FeatureBMI);<br> }<br>+ if ((EBX >> 4) & 0x1) {<br>+ HasHLE = true;<br>+ ToggleFeature(X86::FeatureHLE);<br>+ }<br> if (IsIntel && ((EBX >> 5) & 0x1)) {<br> X86SSELevel = AVX2;<br> ToggleFeature(X86::FeatureAVX2);<br>@@ -439,6 +443,7 @@<br> HasBMI = false;<br> HasBMI2 = false;<br> HasRTM = false;<br>+ HasHLE = false;<br> HasADX = false;<br> IsBTMemSlow = false;<br> IsUAMemFast = false;<br>Index: lib/Target/X86/X86Subtarget.h<br>===================================================================<br>--- lib/Target/X86/X86Subtarget.h<br>+++ lib/Target/X86/X86Subtarget.h<br>@@ -121,6 +121,9 @@<br> /// HasRTM - Processor has RTM instructions.<br> bool HasRTM;<br><br>+ /// HasHLE - Processor has HLE.<br>+ bool HasHLE;<br>+<br> /// HasADX - Processor has ADX instructions.<br> bool HasADX;<br><br>@@ -253,6 +256,7 @@<br> bool hasBMI() const { return HasBMI; }<br> bool hasBMI2() const { return HasBMI2; }<br> bool hasRTM() const { return HasRTM; }<br>+ bool hasHLE() const { return HasHLE; }<br> bool hasADX() const { return HasADX; }<br> bool isBTMemSlow() const { return IsBTMemSlow; }<br> bool isUnalignedMemAccessFast() const { return IsUAMemFast; }<br><D437.2.patch>_______________________________________________<br>llvm-commits mailing list<br>llvm-commits@cs.uiuc.edu<br>http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits<br></blockquote><br><br>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a></blockquote></div></blockquote></div><br></div></body></html>