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

[Daniel Sanders via llvm-commits]

> On 24 Oct 2017, at 10:35, Justin Bogner <mail at justinbogner.com> wrote:
> 
> Daniel Sanders via llvm-commits <llvm-commits at lists.llvm.org <mailto: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?

You're right, it should be checked before this. Thanks for noticing that.

>>       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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171024/7a583e4d/attachment.html>