[llvm] SystemZ: Handle gr128 to fp128 copies in copyPhysReg (PR #90861)

Matt Arsenault via llvm-commits llvm-commits at lists.llvm.org
Wed May 8 07:06:00 PDT 2024


arsenm wrote:

>  I guess you ran into this with an early version of the f128 atomic load/store bitcast patch, but the current version splits into two 64-bit copies very early and never trigger this code path either.

No, I ran into this with the better version (https://github.com/arsenm/llvm-project/tree/systemz-legal-v2i64). v2i64 should be legal, and avoids all of this custom lowering hacking around the SelectionDAG requirement to avoid illegal types. What's pushed now is where I landed after I gave up on making sure none of the ABIs changed 

> 
> Unless I'm missing something here, I'd prefer to revert this PR again rather than keep this as dead code.

copyPhysReg really should be comprehensive. This is tested code so I don't see a compelling reason to remove it and I strongly suggest leaving it. This is avoiding future work for someone 

It's not guaranteed that the DAG never emits these copies. When I was working on the legal type patch, these were slipping in from not-100% precise selection pattern predicates. Having mandatory optimization is bad, and this should always be available as a fallback.

The DAG is also not the only possible source of copies, any machine pass should be able to insert them. GlobalIsel also does away with the "type = register class" DAG reasoning. The fact this was missing also shows there's a missed opportunity for RA to avoid spills by copying to these other classes. This would work if there was a new allocatable synthetic class that was the union of FPR128/GPR128 






https://github.com/llvm/llvm-project/pull/90861


More information about the llvm-commits mailing list