<div dir="ltr">I committed the patch with a comment and test case (r212747).<div><br></div><div>Thank you for the review and feedback.</div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Wed, Jul 9, 2014 at 6:56 PM, Eric Christopher <span dir="ltr"><<a href="mailto:echristo@gmail.com" target="_blank">echristo@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 class="">On Wed, Jul 9, 2014 at 6:54 PM, Bob Wilson <<a href="mailto:bob.wilson@apple.com">bob.wilson@apple.com</a>> wrote:<br>
><br>
>> On Jul 9, 2014, at 6:52 PM, Eric Christopher <<a href="mailto:echristo@gmail.com">echristo@gmail.com</a>> wrote:<br>
>><br>
>> On Wed, Jul 9, 2014 at 6:48 PM, Bob Wilson <<a href="mailto:bob.wilson@apple.com">bob.wilson@apple.com</a>> wrote:<br>
>>><br>
>>>> On Jul 9, 2014, at 6:36 PM, Eric Christopher <<a href="mailto:echristo@gmail.com">echristo@gmail.com</a>> wrote:<br>
>>>><br>
>>>> Might as well add the testcase since we're sure that's the right behavior yes?<br>
>>><br>
>>> Sure, that’s a good idea.<br>
>>><br>
>>>><br>
>>>> Also I'm probably missing something about having the pseudo as<br>
>>>> hasSideEffects = 0 when the instruction the pseudo is representing<br>
>>>> does have side effects. Mind explaining?<br>
>>><br>
>>> It’s defined inside a let-block with Defs = [EFLAGS]. Are there other side effects besides that?<br>
>><br>
>> Ah, I see. No, no other side effects than that, and that's the same<br>
>> for all test instructions on x86. And since hasSideEffects counts the<br>
>> operands and I'm guessing it counts the let Defs = as operands then<br>
>> there are no other side effects. Whee.<br>
>><br>
>> Probably should move the hasSideEffects = 0 up then to the Defs?<br>
><br>
> All of the other instructions there have patterns, so tablegen will infer that they don’t have side effects. TEST8ri_NOEREX is the only one without a pattern.<br>
<br>
</div>Whee.<br>
<br>
Akira: How about a comment then with the testcase?<br>
<span class="HOEnZb"><font color="#888888"><br>
-eric<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
><br>
>><br>
>> -eric<br>
>><br>
>>><br>
>>>><br>
>>>> -eric<br>
>>>><br>
>>>> On Wed, Jul 9, 2014 at 6:27 PM, Bob Wilson <<a href="mailto:bob.wilson@apple.com">bob.wilson@apple.com</a>> wrote:<br>
>>>>> LGTM.<br>
>>>>><br>
>>>>> Even though the original issue no only occurs, the changes here are still the right thing to do. For the record, the original test was:<br>
>>>>><br>
>>>>> target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f\<br>
>>>>> 32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32\<br>
>>>>> :64-S128"<br>
>>>>> target triple = "x86_64-apple-macosx10.9.0"<br>
>>>>><br>
>>>>> define i32 @check_flag(i32 %flags, ...) nounwind {<br>
>>>>> entry:<br>
>>>>> %and = and i32 %flags, 512<br>
>>>>> %tobool = icmp eq i32 %and, 0<br>
>>>>> br i1 %tobool, label %if.end, label %if.then<br>
>>>>><br>
>>>>> if.then: ; preds = %entry<br>
>>>>> br label %if.end<br>
>>>>><br>
>>>>> if.end: ; preds = %entry, %if.then<br>
>>>>> %hasflag = phi i32 [ 1, %if.then ], [ 0, %entry ]<br>
>>>>> ret i32 %hasflag<br>
>>>>> }<br>
>>>>><br>
>>>>>> On Jul 7, 2014, at 3:07 PM, Akira Hatanaka <<a href="mailto:ahatanak@gmail.com">ahatanak@gmail.com</a>> wrote:<br>
>>>>>><br>
>>>>>> This patch marks the pattern-less pseudo-instruction TEST8ri_NOEREX as hasSIdeEffects=0 and adds a case clause for it in X86InstrInfo::shouldScheduleAdjacent to enable macro-fusion.<br>
>>>>>><br>
>>>>>> The original intent of this patch was to fix a bug where a testb instruction was scheduled apart from the je instruction that reads its result (this bug can be reproduced with r192750). It seems that this bug no longer occurs after the node ordering of selection DAG nodes was fixed, so I wasn't able to come up with the a test case.<br>
>>>>>><br>
>>>>>> <test8rinorex1.patch>_______________________________________________<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" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><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" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
>>><br>
><br>
</div></div></blockquote></div><br></div>