None of the .set directives can be used before the .module directives. The .set mips0/pop/push were not triggering this constraint.
Also added testing for all the other implemented directives which are supposed to trigger this constraint.
Details
Diff Detail
Event Timeline
The code change looks correct to me but I think there's a problem with the test.
test/MC/Mips/module-set-directives-bad.s | ||
---|---|---|
1–134 ↗ | (On Diff #19469) | I don't think this test covers what you think it does. It looks like it's only testing that '.set mips0' triggers the errors. All the subsequent errors will appear as a consequence of '.set mips0' regardless of whether those directives do the right thing. I think we need a special directive for testing purposes to reset the flag in between each test (.llvm_internal_reallow_module_directive?). That way we can reset the flag between each subtest. The alternative is to have a separate file (and associated process overhead) for each subtest which seems unreasonable. |
Added internal directive for resetting .module, as suggested in the review.
Added missing call to forbidModuleDirective in emitDirectiveSetFp.
Removed CHECKs for .set macro/nomacro because they don't get emitted at the moment.
Changed the name of the test file to a more appropriate one.
LGTM with the CHECK-NOT's added
test/MC/Mips/module-directive-bad.s | ||
---|---|---|
9 ↗ | (On Diff #21091) | You ought to have a CHECK-NOT on each of these to ensure the error doesn't trigger. |