[llvm] r316463 - [globalisel][tablegen] Multi-insn emission requires that BuildMIAction support not being linked to an InstructionMatcher. NFC

[Justin Bogner via llvm-commits]

Daniel Sanders via llvm-commits <llvm-commits at lists.llvm.org> writes:
> Author: dsanders
> Date: Tue Oct 24 10:08:43 2017
> New Revision: 316463
>
> URL: http://llvm.org/viewvc/llvm-project?rev=316463&view=rev
> Log:
> [globalisel][tablegen] Multi-insn emission requires that BuildMIAction
> support not being linked to an InstructionMatcher. NFC
>
> When multi-instruction emission is supported, it will no longer be guaranteed
> that every BuildMIAction has a corresponding matched instruction. BuildMIAction
> should support not having one to cover the case where a rule produces more
> instructions than it matched.
>
>
> Modified:
>     llvm/trunk/utils/TableGen/GlobalISelEmitter.cpp
>
> Modified: llvm/trunk/utils/TableGen/GlobalISelEmitter.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/GlobalISelEmitter.cpp?rev=316463&r1=316462&r2=316463&view=diff
> ==============================================================================
>
> --- llvm/trunk/utils/TableGen/GlobalISelEmitter.cpp (original)
> +++ llvm/trunk/utils/TableGen/GlobalISelEmitter.cpp Tue Oct 24 10:08:43 2017
> @@ -1753,18 +1753,18 @@ class BuildMIAction : public MatchAction
>  private:
>    unsigned InsnID;
>    const CodeGenInstruction *I;
> -  const InstructionMatcher &Matched;
> +  const InstructionMatcher *Matched;
>    std::vector<std::unique_ptr<OperandRenderer>> OperandRenderers;
>  
>    /// True if the instruction can be built solely by mutating the opcode.
>    bool canMutate(RuleMatcher &Rule) const {
> -    if (OperandRenderers.size() != Matched.getNumOperands())
> +    if (OperandRenderers.size() != Matched->getNumOperands())

Did you mean to check for !Matched here first?

>        return false;
>  
>      for (const auto &Renderer : enumerate(OperandRenderers)) {
>        if (const auto *Copy = dyn_cast<CopyRenderer>(&*Renderer.value())) {
>          const OperandMatcher &OM = Rule.getOperandMatcher(Copy->getSymbolicName());
> -        if (&Matched != &OM.getInstructionMatcher() ||
> +        if ((Matched != nullptr && Matched != &OM.getInstructionMatcher()) ||

If not, you've already executed UB by the time you do this check.

>              OM.getOperandIndex() != Renderer.index())
>            return false;
>        } else
> @@ -1776,7 +1776,7 @@ private:
>  
>  public:
>    BuildMIAction(unsigned InsnID, const CodeGenInstruction *I,
> -                const InstructionMatcher &Matched)
> +                const InstructionMatcher *Matched)
>        : InsnID(InsnID), I(I), Matched(Matched) {}
>  
>    template <class Kind, class... Args>
> @@ -2651,7 +2651,7 @@ Expected<BuildMIAction &> GlobalISelEmit
>      IsExtractSubReg = true;
>    }
>  
> -  auto &DstMIBuilder = M.addAction<BuildMIAction>(0, DstI, InsnMatcher);
> +  auto &DstMIBuilder = M.addAction<BuildMIAction>(0, DstI, &InsnMatcher);
>  
>    // Render the explicit defs.
>    for (unsigned I = 0; I < DstI->Operands.NumDefs; ++I) {
> @@ -2802,7 +2802,7 @@ Expected<RuleMatcher> GlobalISelEmitter:
>        M.defineOperand(OM0.getSymbolicName(), OM0);
>        OM0.addPredicate<RegisterBankOperandMatcher>(RC);
>  
> -      auto &DstMIBuilder = M.addAction<BuildMIAction>(0, &DstI, InsnMatcher);
> +      auto &DstMIBuilder = M.addAction<BuildMIAction>(0, &DstI, &InsnMatcher);
>        DstMIBuilder.addRenderer<CopyRenderer>(0, DstIOperand.Name);
>        DstMIBuilder.addRenderer<CopyRenderer>(0, Dst->getName());
>        M.addAction<ConstrainOperandToRegClassAction>(0, 0, RC);
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits