Prevent renaming of CR fields in AADB when a CR restore is present

[nemanjai]

Summary

A few months ago, a "regression" was introduced in the PPC back end with a change that someone made to target-independent code. Of course, the offending changeset was correct and the issue was when Aggressive Anti-Dependence Breaking gets a hold of CR fields. Namely, it will rename the CR field in the def and all the uses but when restoring the value, the position of the nibble in the source GPR is important. In that case, renaming the CR field is not valid since the MTOCRF will end up populating the field with undefined bits.

[nemanjai]

We should also consider whether we should set the flags on the MTCRF/MFCRF instructions. There currently does not appear to be a need for that since the only way we emit that instruction is in the assembly output (PPCAsmPrinter.cpp). However, if we decide to use that instruction in the future to save/restore multiple CR fields at a time, we should consider the effect that AADB will have on it.

[kbarton]

LGTM

[hfinkel]

In D15930#320646, @nemanjai wrote:

We should also consider whether we should set the flags on the MTCRF/MFCRF instructions. There currently does not appear to be a need for that since the only way we emit that instruction is in the assembly output (PPCAsmPrinter.cpp). However, if we decide to use that instruction in the future to save/restore multiple CR fields at a time, we should consider the effect that AADB will have on it.

Yes, please set it on mtcrf and mfcrf (hasExtraSrcRegAllocReq on the latter). We always set instruction flags based on theoretical considerations, not based on how they're actually generated (as that might always change in the future).

lib/Target/PowerPC/PPCInstr64Bit.td

302

If you're going to add a comment, and I agree doing so is valuable, we should make it more explanatory. How about this:

// mtocrf's input needs to be prepared by shifting by an amount dependent on the cr register selected. Thus, post-ra anti-dep breaking must not later change that register assignment.

[nemanjai]

Addressed the comments and committed.
Committed revision 257168.

lib/Target/PowerPC/PPCInstr64Bit.td

 // 64-bit CR instructions
 let Interpretation64Bit = 1, isCodeGenOnly = 1 in {
 let hasSideEffects = 0 in {
+let hasExtraDefRegAllocReq = 1 in // to enable post-ra anti-dep breaking.
 def MTOCRF8: XFXForm_5a<31, 144, (outs crbitm:$FXM), (ins g8rc:$ST),
                         "mtocrf $FXM, $ST", IIC_BrMCRX>,
             PPC970_DGroup_First, PPC970_Unit_CRU;

lib/Target/PowerPC/PPCInstrInfo.td

                      "#RESTORE_VRSAVE", []>;
 
 let hasSideEffects = 0 in {
+let hasExtraDefRegAllocReq = 1 in // to enable post-ra anti-dep breaking.
 def MTOCRF: XFXForm_5a<31, 144, (outs crbitm:$FXM), (ins gprc:$ST),
                        "mtocrf $FXM, $ST", IIC_BrMCRX>,
             PPC970_DGroup_First, PPC970_Unit_CRU;