User Details
- User Since
- Nov 24 2016, 5:21 AM (330 w, 3 d)
Fri, Mar 24
Wed, Mar 22
Sorry, I've not had much review time this week but I'll take a proper look tomorrow or Friday.
Fri, Mar 17
Thu, Mar 16
To prevent some duplication of event I just wanted to update my previous comment and say all the dependent patches have now landed and I'm planning to add _u intrinsics/builtins for the integer MLA instructions soon.
Wed, Mar 15
Ping now the work to disallow vscale constant expressions is complete (D145404).
This option was our pragmatic way to ensure scalable vector based toolchains remained useful whilst the kinks were worked out. We've a few releases under our belts now and I feel that enough works to consider further errors as something we should not be hiding.
Tue, Mar 14
We knew when landing the original patch that it'll likely bite us, but at that time there just wasn't a good enough reason to warrant the loss in code quality.
Mon, Mar 13
Sun, Mar 12
Fri, Mar 10
Thu, Mar 9
ping
Wed, Mar 8
Tue, Mar 7
Mon, Mar 6
@cameron.mcinally : Given D141924 can this patch be abandoned?
Restored simplification for case where isSupportedGetElementPtr() is false.
A few recommended improvements but otherwise looks good.
Tighten up a couple of function signatures.
I suspect we'll want to replace replace-intrinsics-with-veclib-sqrt.ll with more exhaustive testing for all the intrinsics but that's a job for another day. I've a couple of observations but otherwise the patch looks good.
Fair enough. Sorry for the misunderstanding.
Fri, Mar 3
Rebase after updating and precommiting the tests.
Add matching combine for "A | (~A & B) --> A | B".
Thu, Mar 2
Thanks for the education @nikic. I'll implement that transformation as well.
Add scalar test cases and fixed switch operand test.
This is a partial fix for https://github.com/llvm/llvm-project/issues/58690
Updated to include correct handling of poison.
Need to be more strict with the handling of poison.
ping
Not a strongly held view but is there value in restricting the combine to pre-operation legalisation? I ask because I wonder if they'll be a point during legalisation where somebody specifically wants the ISD::VSCALE node. Perhaps a different/better option is to move the logic into SelectionDAG::getVScale(), which can take a default false bool to disable the optimisation?
Wed, Mar 1
A couple of minor issues but otherwise looks good.
Tue, Feb 28
Mon, Feb 27
This patch follows on from a conversation on D144624. I've tried to keep things conservative (i.e. no llvm.vscale() detection or use of vscale_range attributes) so as not to change existing behaviour, hence no test changes.
Feb 24 2023
One minor suggestion but otherwise looks good.
Feb 23 2023
Thanks for the information @nikic . I'm assuming the road to ptradd will not be short? so I'll investigate adding something to SCEV so we can move towards updating the LangRef for vscale.
I realise this patch is partly refactoring but it does seem to increase the value of scalable vector GEP ConstantExprs, which is inconsistent to your view on D134648? Is it possible for SCEV to have its own representation of vscale that is independent to the Constant class hierarchy?
Feb 22 2023
Feb 21 2023
Feb 20 2023
There's likely other combines that will also need to be updated but this is the most important one blocking D143767. I'd like to take a more holistic look at the others as part of work to unify some code paths. For example, I want to canonicalise sve intrinsic calls where the predicate is all active to use the "undef" intrinsics so that some code duplication within the code generator can be removed. With that said, please shout if you know of something critical that should be handled before D143767 lands.