Skip tests which rely on Mips backend. #733
Conversation
veselypeta
force-pushed
the
petr/break-mips-riscv-dependancy
branch
from
f6cafcf
to
c578e69
Compare
|
@arichardson Does this seem sensible to you? |
llvm/test/Transforms/CheriBoundAllocas/pg_lzcompress-stack-bounds-crash.ll
Show resolved
Hide resolved
| @@ -42,7 +43,8 @@ void test_onstack_int_overaligned(struct Foo *s) { | |||
|
|
|||
| void test__stack_array(void) { | |||
| char buf[333]; | |||
| do_stuff_with_buf(buf); // CSV-NEXT: 0,333,?,"{{.+}}/csetbounds-stats-all.cpp:45:21","Add subobject bounds","array decay for char[333]" | |||
| do_stuff_with_buf(buf); | |||
There was a problem hiding this comment.
The thing here is that the line number 45 is hard coded, by adding the // REQUIRES: cause causes this test to fail. All the other assertions use the [[@LINE]] directive to get the line making this clause an outlier. Is there a specific reason why this assertion needs to be this way?
veselypeta
force-pushed
the
petr/break-mips-riscv-dependancy
branch
from
c578e69
to
fd7059e
Compare
|
We are still building the Mips backend in order to leverage the Mips testing for testing any CHERI changes. However, I still think this change is useful as you can still build a RISCV target without CHERI and still have all the tests passing. |
| @@ -81,6 +81,9 @@ def __init__(self): | |||
| parser = argparse.ArgumentParser() | |||
| parser.add_argument("--llvm-bindir", type=Path, required=True) | |||
| parser.add_argument("--verbose", action="store_true") | |||
| parser.add_argument("--arch", default='all', | |||
There was a problem hiding this comment.
See https://git.morello-project.org/morello/llvm-project/-/commit/9ce8358caa77e9d71fe0278b639070bdcbad7ca3 for Morello which looks at the supported targets.
| @@ -1,3 +1,4 @@ | |||
| // REQUIRES: mips-registered-target | |||
There was a problem hiding this comment.
Should not be needed for Driver tests. I wonder which line fails?
There was a problem hiding this comment.
Technically yes, but in this case clang forwards the -mxcaptable parameter to the backend, which is only supported by mips.
https://github.com/CTSRD-CHERI/llvm-project/blob/master/clang/lib/Driver/ToolChains/Clang.cpp#L1964
https://github.com/CTSRD-CHERI/llvm-project/blob/master/llvm/lib/Target/Mips/MipsISelLowering.cpp#L97
There was a problem hiding this comment.
Ah and if that file is not built then the flag is not accepted. Should probably just modify the test to not actually consume the flag but just check the generated command line.
| @@ -1,3 +1,4 @@ | |||
| // REQUIRES: mips-registered-target | |||
There was a problem hiding this comment.
Shouldn't need a registered target?
There was a problem hiding this comment.
Same issue as below with -mxcaptable
There was a problem hiding this comment.
Hmm this may be slightly annoying to fix properly, so just adding this requires seems fine to me.
There was a problem hiding this comment.
Well, this test isn't really a Driver test, as it's using -emit-llvm
There was a problem hiding this comment.
That's fair, ideally -emit-llvm should be removed. Maybe the flag validation only happens in the backend but it's been a long time since I looked at that code. I wonder if -E also verifies it?
There was a problem hiding this comment.
I could be wrong here. But the tests here invoke clang directly, which invokes the driver too. If we're just testing frontend code I think these tests should be modified to only invoke clang -cc1
For the script to succeed it requires that MIPS & RISCV targets are registered. The --arch option allows the script to run only against the targets which you're building for.
veselypeta
force-pushed
the
petr/break-mips-riscv-dependancy
branch
from
2975066
to
cbc5ba1
Compare
Currently, the Mips target is required to be built for many cheri tests to pass even if only targeting RISCV. This PR annotates all tests which rely on the Mips backend with the
REQUIRESkeyword in order to successfully skip over them without reporting as a failure. Additionally I've added a--archoption toregenerate-all.pyin order to allow you to regenerate these tests for only the specific architecture you're targetting.