Diff Detail
- Repository
- rG LLVM Github Monorepo
Unit Tests
Time | Test | |
---|---|---|
60,050 ms | x64 debian > libFuzzer.libFuzzer::minimize_crash.test |
Event Timeline
I see we already have test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.intersect_ray.ll. Is that redundant now? (Generally I think we should combine sdag/gisel codegen tests in a single file, as you have done here, rather than having separate files.)
The only differences between the tests discovered are the use of inreg and flattened arguments instead of vector ones in SDAG tests. I assumed that's what we prefer for the combined tests.
Generally we would not bother with inreg except for instruction operands that have to go in SGPRs. But it doesn't really matter. At worst, it will bloat the output with some extra SGPR-to-VGPR copies.
and flattened arguments instead of vector ones in SDAG tests. I assumed that's what we prefer for the combined tests.
Again it doesn't really matter. Personally I would use vector arguments because it keeps the IR nice and short.
No no, all the permutations should be tested. This is not a cosmetic choice, we should be testing these are handled correctly
No no, all the permutations should be tested. This is not a cosmetic choice, we should be testing these are handled correctly
That's what the comments say, yes. But then if vector versions have only been used to make sure the register operands are where the values actually land, maybe we can test that just by adding some extra arguments to flattened versions and thus make accidentally correct output sufficiently unlikely?
Tests like this don't feel to be the right place to test codegen'ing of moves preparing the operands and cluttering tests checking them repeatedly for various kinds of calls probably doesn't make much sense as well?
@arsenm Matt, are you fine with this as-is? If not, would adding more parameters as described in https://reviews.llvm.org/D142375#4132791 be enough?
I don't really follow what this is saying. One reason for the split would have been that SelectionDAG is broken in the trickier cases that require waterfall loops
Tests like this don't feel to be the right place to test codegen'ing of moves preparing the operands and cluttering tests checking them repeatedly for various kinds of calls probably doesn't make much sense as well?
It's making sure they codegen for any inputs, which would also cover the waterfall cases?
I still think all the S/V permutations should be covered in these intrinsic tests. The benefit of merging the tests is not worth losing coverage to work around the DAG not working