@@ -6913,7 +6913,8 @@ defm PCMPESTRI : SS42AI_pcmpestri<"pcmpestri">;<br> // crc intrinsic instruction<br> // This set of instructions are only rm, the only difference is the size<br> // of r and m.<br>-let Constraints = "$src1 = $dst" in {<br>
+let Constraints = "$src1 = $dst",<br>+ Predicates = [HasSSE42] in {<br> def CRC32r32m8 : SS42FI<0xF0, MRMSrcMem, (outs GR32:$dst),<br> (ins GR32:$src1, i8mem:$src2),<br> "crc32{b} \t{$src2, $src1|$src1, $src2}",<br>
<br>This isn't necessary. SS42FI is only used by CRC32.<br><br>@@ -6804,7 +6804,7 @@ multiclass pseudo_pcmpistrm<string asm> {<br> let Defs = [EFLAGS], usesCustomInserter = 1 in {<br> let AddedComplexity = 1 in<br>
defm VPCMPISTRM128 : pseudo_pcmpistrm<"#VPCMPISTRM128">, Requires<[HasAVX]>;<br>- defm PCMPISTRM128 : pseudo_pcmpistrm<"#PCMPISTRM128">, Requires<[HasSSE42]>;<br>+ defm PCMPISTRM128 : pseudo_pcmpistrm<"#PCMPISTRM128">, Requires<[UseSSE42]>;<br>
}<br> <br> let Defs = [XMM0, EFLAGS], neverHasSideEffects = 1, Predicates = [HasAVX] in {<br>@@ -6842,7 +6842,7 @@ multiclass pseudo_pcmpestrm<string asm> {<br> let Defs = [EFLAGS], Uses = [EAX, EDX], usesCustomInserter = 1 in {<br>
let AddedComplexity = 1 in<br> defm VPCMPESTRM128 : pseudo_pcmpestrm<"#VPCMPESTRM128">, Requires<[HasAVX]>;<br>- defm PCMPESTRM128 : pseudo_pcmpestrm<"#PCMPESTRM128">, Requires<[HasSSE42]>;<br>
+ defm PCMPESTRM128 : pseudo_pcmpestrm<"#PCMPESTRM128">, Requires<[UseSSE42]>;<br> }<br> <br> let Predicates = [HasAVX],<br><br>Can you remove the AddedComplexity = 1 here?<br><br>Other than those two things. I think it looks good.<br>
<br><div class="gmail_quote">On Wed, Aug 29, 2012 at 8:32 PM, Craig Topper <span dir="ltr"><<a href="mailto:craig.topper@gmail.com" target="_blank">craig.topper@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
I'm in agreement with the plan. Haven't done a detailed review of the patch yet. Hopefully I can get to that later tonight.<div class="HOEnZb"><div class="h5"><br><br><div class="gmail_quote">On Wed, Aug 29, 2012 at 4:21 PM, Eli Friedman <span dir="ltr"><<a href="mailto:eli.friedman@gmail.com" target="_blank">eli.friedman@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div>On Tue, Aug 28, 2012 at 11:25 AM, Michael Liao <<a href="mailto:michael.liao@intel.com" target="_blank">michael.liao@intel.com</a>> wrote:<br>
> Ping again for comment on this proposal. - Michael<br>
><br>
> On Sun, 2012-08-26 at 02:57 -0700, Michael Liao wrote:<br>
>> On Fri, 2012-08-24 at 19:23 -0700, Eli Friedman wrote:<br>
>> > On Fri, Aug 24, 2012 at 6:55 PM, Michael Liao <<a href="mailto:michael.liao@intel.com" target="_blank">michael.liao@intel.com</a>> wrote:<br>
>> > > On Fri, 2012-08-24 at 18:14 -0700, Eli Friedman wrote:<br>
>> > >> On Wed, Aug 22, 2012 at 10:27 AM, Michael Liao <<a href="mailto:michael.liao@intel.com" target="_blank">michael.liao@intel.com</a>> wrote:<br>
>> > >> > Hi<br>
>> > >> ><br>
>> > >> > The attached patch adds several new predicates, namely UseSSE1, UseSSE2,<br>
>> > >> > UseSSE3, UseSSSE3, UseSSE41, and UseSSE42.<br>
>> > >> ><br>
>> > >> > As the penalty of inter-mixing SSE and AVX instructions, we need prevent<br>
>> > >> > SSE legacy insn from being generated except explicitly specified through<br>
>> > >> > some intrinsics. For patterns both supported by both SSE and AVX, so<br>
>> > >> > far, we force AVX insn will be tried first relying on AddedComplexity or<br>
>> > >> > td location. It's error-prone and introduces bugs accidentally.<br>
>> > >> ><br>
>> > >> > 'UseSSEx' is disabled when AVX is turned on. For insns both supported by<br>
>> > >> > AVX and SSE insns, we need this predicate to force VEX encoding only.<br>
>> > >> ><br>
>> > >> > For insns not inherited by AVX, we still use the previous predicates,<br>
>> > >> > i.e. 'HasSSEx'. So far, these insns fall into the following categories:<br>
>> > >> > * SSE insns with MMX operands<br>
>> > >> > * SSE insns with GPR/mem operands only (xFENCE, PREFETCH, CLFLUSH,<br>
>> > >> > CRC, and etc.)<br>
>> > >> > * SSE4A insns.<br>
>> > >> > * MMX insns.<br>
>> > >> > * x87 insns added by SSE.<br>
>> > >> ><br>
>> > >> > With this patch, several inter-mixing cases are found and fixed from<br>
>> > >> > regression tests.<br>
>> > >><br>
>> > >> --- a/test/ExecutionEngine/MCJIT/test-common-symbols.ll<br>
>> > >> +++ b/test/ExecutionEngine/MCJIT/test-common-symbols.ll<br>
>> > >> @@ -1,4 +1,4 @@<br>
>> > >> -; RUN: %lli -use-mcjit -O0 -disable-lazy-compilation=false %s<br>
>> > >> +; RUN: %lli -use-mcjit -disable-lazy-compilation=false %s<br>
>> > >><br>
>> > >> ; The intention of this test is to verify that symbols mapped to COMMON in ELF<br>
>> > >> ; work as expected.<br>
>> > >><br>
>> > >> What is the point of this change?<br>
>> > ><br>
>> > > So far, MCJIT cannot support the creation of '.rodata' section. This's<br>
>> > > work around. I will file a bug for MCJIT.<br>
>> ><br>
>> > Oh, I see...<br>
>> ><br>
>> > How hard would it be to fix fast-isel so it can select vcvtsi2sdq?<br>
>><br>
>> Regard to the failure of MCJIT with this patch, finally I find the root<br>
>> cause. Target constant rematerialization in X86 fast-isel doesn't handle<br>
>> large code model correctly and create a PC-rel addressing which crashes<br>
>> when the displacement is out of 32-bit immediate range. I added a simple<br>
>> fix similar to large code model is handled in X86 fast-isel in<br>
>> elsewhere.<br>
>><br>
>> The patch is revised and attached here. Not sure it's ok to commit.<br>
<br>
</div></div>The fast-isel changes look fine; please commit as a separate patch<br>
with its own testcase.<br>
<br>
The other changes also look reasonable, but I would prefer if someone<br>
else familiar with AVX could take a look as well.<br>
<br>
Please file a bug on the fast-isel floating-point constant issue once<br>
you commit this.<br>
<div><div><br>
-Eli<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</div></div></blockquote></div><br><br clear="all"><br></div></div><span class="HOEnZb"><font color="#888888">-- <br>~Craig<br>
</font></span></blockquote></div><br><br clear="all"><br>-- <br>~Craig<br>