[SpeculationPass] Fixed the enable signal of speculator

[shundroid]

Background: The speculator needs a signal to determine the timing of the speculation, just as constant units need a control signal. The speculator makes a speculation every time the BB it belongs to is triggered; thus the signal should be routed from the speculator's BB itself so that it indicates the start of the BB.

Before: the enable signal of the speculator was routed from the previous BB (bug). After: it will be from the control network in the same BB, just before CBranch.

And the name of the former enable signal is changed to "trigger signal".

(If I should place a figure here I will do that. It will be similar to Fig. 34 of Haoran's thesis)

[murphe67]

I haven't read the mechanism you use to place it below the buffer, but could we structure it so that the fork is placed directly above the output cbranch? We want the fork as near the cbranch as possible so we guarantee synchronization at its inputs. I don't think we need to make assumptions about where the buffer is, just where its not:

[shundroid]

Thank you for your suggestion and it seems more elegant. So I will change the traverse from the cmerge to just pointing out above the cbranch.

[shundroid]

I updated the implementation so please check it. And still replaceAllUsesExcept cannot be used to place units so I'll use operand->set. I think I should describe the reason in detail and I'll draw a figure.

[shundroid]

This is why replaceAllUsesExcept doesn't work for the placement of the save-commit unit

(a) control network (possibly) buffer to cbranch (b) connect the enable signal to the speculator using the same Value (non-canonicalized) (c)-(e) how the placement of the save-commit unit is conducted if we use replaceAllUsesExcept

[murphe67]

Just double checking I understand the change:

[shundroid]

That's right! If we place speculator after the save-commit unit it will cause a deadlock

[shundroid]

I've fixed the part you mentioned and added comments. Please take a look!

[shundroid]

@emmet-murphy there was a case where a BB doesn't have either cmerge or cbranch In this case we need to check BBArc. I'll change the implementation

[shundroid]

We decided to target only the condition speculation for now, so it is guaranteed that the speculator BB has a cbranch. I'll make the error clarifying this point.

[shundroid]

@emmet-murphy I'd be glad if you could review this!!

[murphe67]

This looks great Shun! Once we decide on the bb number access, we can ask for a final review

[murphe67]

yeah Jiahui convinced me we should rename "specEnable" to something like "specTrigger" throughout the entire flow/documentation

[shundroid]

I fixed all three points. Please take a look!