[PATCH][x86] Mark TEST8ri_NOREX hasSideEffects=0

[Bob Wilson]

> On Jul 9, 2014, at 6:52 PM, Eric Christopher <echristo at gmail.com> wrote:
> 
> On Wed, Jul 9, 2014 at 6:48 PM, Bob Wilson <bob.wilson at apple.com> wrote:
>> 
>>> On Jul 9, 2014, at 6:36 PM, Eric Christopher <echristo at gmail.com> wrote:
>>> 
>>> Might as well add the testcase since we're sure that's the right behavior yes?
>> 
>> Sure, that’s a good idea.
>> 
>>> 
>>> Also I'm probably missing something about having the pseudo as
>>> hasSideEffects = 0 when the instruction the pseudo is representing
>>> does have side effects. Mind explaining?
>> 
>> It’s defined inside a let-block with Defs = [EFLAGS]. Are there other side effects besides that?
> 
> Ah, I see. No, no other side effects than that, and that's the same
> for all test instructions on x86. And since hasSideEffects counts the
> operands and I'm guessing it counts the let Defs = as operands then
> there are no other side effects. Whee.
> 
> Probably should move the hasSideEffects = 0 up then to the Defs?

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.

> 
> -eric
> 
>> 
>>> 
>>> -eric
>>> 
>>> On Wed, Jul 9, 2014 at 6:27 PM, Bob Wilson <bob.wilson at apple.com> wrote:
>>>> LGTM.
>>>> 
>>>> 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:
>>>> 
>>>> target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f\
>>>> 32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32\
>>>> :64-S128"
>>>> target triple = "x86_64-apple-macosx10.9.0"
>>>> 
>>>> define i32 @check_flag(i32 %flags, ...) nounwind {
>>>> entry:
>>>> %and = and i32 %flags, 512
>>>> %tobool = icmp eq i32 %and, 0
>>>> br i1 %tobool, label %if.end, label %if.then
>>>> 
>>>> if.then:                                          ; preds = %entry
>>>> br label %if.end
>>>> 
>>>> if.end:                                           ; preds = %entry, %if.then
>>>> %hasflag = phi i32 [ 1, %if.then ], [ 0, %entry ]
>>>> ret i32 %hasflag
>>>> }
>>>> 
>>>>> On Jul 7, 2014, at 3:07 PM, Akira Hatanaka <ahatanak at gmail.com> wrote:
>>>>> 
>>>>> 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.
>>>>> 
>>>>> 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.
>>>>> 
>>>>> <test8rinorex1.patch>_______________________________________________
>>>>> llvm-commits mailing list
>>>>> llvm-commits at cs.uiuc.edu
>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>> 
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>