[LLVMdev] Artificial deps and stores

[Jakob Stoklund Olesen]

On Jan 17, 2014, at 4:36 PM, Andrew Trick <atrick at apple.com> wrote:

> 
> On Jan 17, 2014, at 4:16 PM, Jakob Stoklund Olesen <stoklund at 2pi.dk> wrote:
> 
>> 
>> On Jan 17, 2014, at 4:03 PM, Andrew Trick <atrick at apple.com> wrote:
>> 
>>> 
>>> On Jan 17, 2014, at 3:54 PM, Hal Finkel <hfinkel at anl.gov> wrote:
>>> 
>>>> Andy, et al.,
>>>> 
>>>> In ScheduleDAGInstrs::buildSchedGraph, the code for handling stores has this:
>>>> 
>>>>    if (!ExitSU.isPred(SU))
>>>>      // Push store's up a bit to avoid them getting in between cmp
>>>>      // and branches.
>>>>      ExitSU.addPred(SDep(SU, SDep::Artificial));
>>>> 
>>>> This code does not seem to be in any way specific to compares; and in any case, at least on the PPC A2, scheduling stores in between the compare and the branch would not be a bad thing (because the core is in order, and the compare has a 2-cycle latency, so if there is nothing else to do, a store would not be a bad thing to put there).
>>>> 
>>>> Can you explain the motivation for this (why or for what it is bad), and what else it might do (aside from the commented cmp/branch pairing)? I'm wondering if we should make this target dependent.
>>> 
>>> I don’t agree with the existing comment. It’s possible that somewhere, maybe in target specific code, we make use of the extra store->exit edge, but I can’t remember any reason for it now.
>> 
>> Do you have another mechanism for encouraging macro fusion?
> 
> The MI scheduler does it now via a proper mechanism. However, for targets that haven’t switched over it could be a minor, temporary issue (the old mechanism shown above didn’t work well anyway).
> 
> For the time being it is probably best to leave it under a target option with a FIXME to avoid perturbing non-PPC/x86 targets.

Since it doesn’t work well anyway, I’d say just remove it.

Thanks,
/jakob

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140117/078656a4/attachment.html>