[PATCH] D49310: [llvm-mca][BtVer2] Teach how to identify dependency-breaking idioms.

[Andrea Di Biagio via Phabricator via llvm-commits]

andreadb added inline comments.


================
Comment at: include/llvm/MC/MCInstrAnalysis.h:97
+  /// dependency breaking instruction doesn't wait on the input register, since
+  /// the result is not dependent on its value.
+  virtual bool isDependencyBreaking(const MCSubtargetInfo &STI,
----------------
RKSimon wrote:
> I don't know if this description is too x86 specific or not - AFAICT the actual 'same register' test is inside X86MCInstrAnalysis::isDependencyBreaking so it isn't a generic requirement?
> 
> An alternative approach could be for MCInstrAnalysis::isDependencyBreaking to return true if the instruction is independent of some/all of its input operands and a APInt& Mask (or similar) is used to list the operands that it doesn't depend upon - not sure if we need all that functionality or not though so it might be over-engineering it.
> 
> Also, do you know if we can make use of this in the machine schedulers or whatever? I'm not against it being inside MCInstrAnalysis in general although the fact that its so cpu-target specific suggests it could get bulky. @craig.topper What do you think?
I am okay with either approach. We can use an APInt to identify register reads that can be bypassed.

If we want to use this knowledge in the machine schedulers, then we can add a hook to the InstructionInfo interface. Each target can then describe dependency breaking instructions using a similar logic.
However, that new hook would take a `MachineInstr` in input, and not a `MCInst`.
So, we still have to expose that information to MC.

We could use the new scheduling predicates to construct a TII predicate. That predicate is translated by tablegen into a method to the TII interface. We can then expand that same predicate into a method in MCInstrAnalysis. That should be doable. I wanted to avoid this scenario. However, if the plan is to reuse this knowledge elsewhere, then I can revisit this patch.



https://reviews.llvm.org/D49310