- User Since
- Nov 20 2019, 6:41 AM (32 w, 5 d)
The patch overall looks good to me - just a question about the assert!
Fri, Jul 3
Thu, Jul 2
LGTM, but good to let other reviewers have a look too!
Wed, Jul 1
These tests will currently fail as they depend upon the child revisions being committed.
- Added extra check that SCALAR_TO_VECTOR result type is also a fixed length vector.
- Actually added proper CHECK lines to my new test. Sorry about that @efriedma - that was a silly mistake.
Tue, Jun 30
Hi @efriedma, sorry yes I forgot to mention in the ticket it fixes an existing warnings in these tests:
Mon, Jun 29
Looks like it has already been fixed by commit https://reviews.llvm.org/D82061
Fri, Jun 26
Please note that I tried my best to find tests that exposed the change in behaviour where I added the fixed length vector check, but to no avail! I put that extra check in anyway as I think the optimisation makes no sense for scalable vector types. All we're doing here is removing an infrequently used (I looked for fixed length vector types hitting this path and there weren't many) optimisation for scalable vector types, so I think so long as no existing tests break it should be ok.
Can you remove the duplicate tests before submitting? Otherwise LGTM!
Thu, Jun 25
Hi @paulwalker-arm, ok fair enough. It was just because you said "sve-fixed-length-int-arith.ll" fails due to missing D82466, but there are 4 tests failing. Just making sure these are all due to D82466 that's all. :)
Hi @paulwalker-arm, are you expecting those four tests to fail on this patch?
Wed, Jun 24
Tue, Jun 23
Perhaps this change is good enough? We call computeValueLLTs just before lowerFormalArguments, which will detect the scalable type and fall back anyway even if the LLT type is wrong. The other option is to add yet another fallBackOnDAGISel type callback that allows the backend to reject certain types before we create the vregs.
This problem occurs in the IRTranslator just before we call lowerFormalArguments (which is currently the earliest point we detect scalable types and fall back to DAG ISel)
#7 0x0000ffffa0f26760 llvm::getLLTForType(llvm::Type&, llvm::DataLayout const&) (/lib64/libc.so.6+0x36760)
#8 0x0000000002556f74 llvm::computeValueLLTs(llvm::DataLayout const&, llvm::Type&, llvm::SmallVectorImpl<llvm::LLT>&, llvm::SmallVectorImpl<unsigned long>*, unsigned long) (.localalias) (./bin/llc+0x2556f74)
#9 0x0000000001aeb108 llvm::IRTranslator::getOrCreateVRegs(llvm::Value const&) (.part.0) (./bin/llc+0x1aeb108)
#10 0x00000000019df8a4 llvm::IRTranslator::runOnMachineFunction(llvm::MachineFunction&) (./bin/llc+0x19df8a4)
Mon, Jun 22
Sun, Jun 21
Fri, Jun 19
- Added full CHECK lines for new test