<div dir="ltr">Hi Quentin,<div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div style="word-wrap:break-word"><div><br>Going back to the concern that we are conflating isRematerializable and isAsCheapAsAMove, I can see why we want to keep both. That said, I still think that if we are overriding isAsCheapAsMove with this new hook for isRematerializable instructions, those instructions should be marked as isAsCheapAsMove too.</div>
<div><br></div><div>This means two things:</div><div>1. All the uses of MachineInstr::isAsCheapAsMove should be promoted to use of TargetInstrInfo::isAsCheapAsMove.</div><div>2. If an instruction is rematerializable but not as cheap as move, then it shouldn’t appear in the isAsCheapAsMove target hook.</div>
<div><br></div><div>If you have an instruction matching #2 and still want to make it appear in TargetInstrInfo::isAsCheapAsMove, it means you are trying to abuse the register coalescer to do rematerialization and that’s probably not the right solution.</div>
<div><br></div><div>What do you think?</div></div></blockquote><div><br></div><div>For case #2, you are right, so if we want to make make it rematerializable, actually we could override isReallyTriviallyReMaterializable(MI, AA) instead.</div>
<div><br></div><div><div>    /// isTriviallyReMaterializable - Return true if the instruction is trivially</div><div>    /// rematerializable, meaning it has no side effects and requires no operands                                                                                                                                                              </div>
<div>    /// that aren't always available.</div><div>    bool isTriviallyReMaterializable(const MachineInstr *MI,</div><div>                                     AliasAnalysis *AA = nullptr) const {</div><div>      return MI->getOpcode() == TargetOpcode::IMPLICIT_DEF ||</div>
<div>              (MI->getDesc().isRematerializable() &&</div><div>              (isReallyTriviallyReMaterializable(MI, AA) ||</div><div>               isReallyTriviallyReMaterializableGeneric(MI, AA)));</div>
<div>    }</div></div><div><br></div><div>For register coalesce pass, so far the algorithm could only rematerialize instructions that are as cheap as move, because of the code in register coalesce pass I pasted previously,</div>
<div><br></div><div><div>  if (!DefMI->isAsCheapAsAMove())</div><div>    return false;</div><div>  if (!TII->isTriviallyReMaterializable(DefMI, AA))                                                                                                                                                                                            </div>
<div>    return false;</div></div><div><br></div><div>If we want to change it, I'd suggest doing it separately.</div><div><br></div><div>If we add flag isAsCheapAsMove in .td file, we needn't to override TargetInstrInfo::isAsCheapAsMove() at all for this patch. Previously I proposed this solution at <a href="http://comments.gmane.org/gmane.comp.compilers.llvm.cvs/194325">http://comments.gmane.org/gmane.comp.compilers.llvm.cvs/194325</a>, and Tim gave me feedback of not adding micro-architecture dependent things like this in .td file, and I was convinced this is a good design level choice. After all, for other micro-architectures, the instructions I'm highlighting right now may not be as cheap as move.</div>
<div><br></div><div>Attached is the patch making the small change as you suggested.</div><div><br></div><div>Thanks,</div><div>-Jiangning</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div style="word-wrap:break-word"><div><br></div><div>Cheers,</div><div>-Quentin</div><div><div class="h5"><div><br></div></div></div></div></blockquote></div><br></div></div>