[PATCH] D25618: Check that emitted instructions meet their predicates on all targets except ARM, Mips, and X86.
Daniel Sanders via llvm-commits
llvm-commits at lists.llvm.org
Fri Oct 14 07:56:38 PDT 2016
dsanders added a comment.
Thanks for the quick feedback.
================
Comment at: lib/Target/AArch64/MCTargetDesc/AArch64MCCodeEmitter.cpp:175
+private:
+ uint64_t ComputeAvailableFeatures(const FeatureBitset &FB) const;
+ void verifyInstructionPredicates(const MCInst &MI,
----------------
jmolloy wrote:
> lowerCamelCase
I agree it should be a lower case first letter. The reason it isn't is because the generated function is shared with the AsmMatcher and that one isn't a member function. I'll fix this.
================
Comment at: lib/Target/AMDGPU/VOP1Instructions.td:539
let VOP1 = 1;
+ let SubtargetPredicate = isGCN;
}
----------------
tstellarAMD wrote:
> dsanders wrote:
> > @tstellarAMD: Hi Tom, I'm not sure if this line is correct or not. I've included it because a recent change (r284031) left a '?' in this field and that causes problems in tablegen. Is there a correct predicate to use here?
> I think this needs to be isVI.
Thanks, I'll update the patch.
================
Comment at: utils/TableGen/CodeEmitterGen.cpp:347
+ o << " assert((AvailableFeatures & RequiredFeatures[Inst.getOpcode()])\n"
+ << " == RequiredFeatures[Inst.getOpcode()] &&\n"
+ << " \"Attempting to emit an instruction that doesn't satisfy the required\"\n"
----------------
jmolloy wrote:
> This absolutely needs to print out the offending instruction, and ideally the required features although as they're encoded in a bitset that's probably quite difficult.
I agree, so far I've been re-running the failing command under lldb, printing Inst.getOpcode() and then looking that up in the generated AsmMatcher but that's a bit long winded.
In the worst case I can generate a table but I'll see if I can re-use existing information first.
https://reviews.llvm.org/D25618
More information about the llvm-commits
mailing list