[PATCH] D16291: AArch64: Implement missed conditional compare sequences.
Balaram Makam via llvm-commits
llvm-commits at lists.llvm.org
Thu Feb 4 08:59:58 PST 2016
Without making it LegalOrCustom it will break these lit-tests:
Would it help if we add a check to let legalize expand this if it isn't legal type yet during cutom lowering?
Adding something like in tryLowerToAArch64Cmp:
From: mmaggioni at apple.com [mailto:mmaggioni at apple.com]
Sent: Wednesday, February 03, 2016 8:12 PM
To: reviews+D16291+public+b25ae380e8516781 at reviews.llvm.org; Balaram Makam
Cc: james.molloy at arm.com; t.p.northover at gmail.com; mcrosier at codeaurora.org; matze at braunis.de; junbuml at codeaurora.org; mssimpso at codeaurora.org; Bbbbb
Subject: Re: [PATCH] D16291: AArch64: Implement missed conditional compare sequences.
I’m experiencing problems after this commit.
The changes the check for OR formation in the DAGCombiner from:
if ((!LegalOperations || TLI.isOperationLegal(ISD::OR, VT)) &&
if ((!LegalOperations || TLI.isOperationLegalOrCustom(ISD::OR, VT)) &&
For us 64-bit ORs are not legal, so we have custom lowering transforming them into ADDs This combine turns them back to ORs creating a cycle in the Combiner that never terminates.
What’s the reasoning behind this change here? Making it LegalOrCustom could actually endup in a problem like this if the lowering for ORs happens to be turning into ADD and goes against the principle that usually Legal things are cheap and Custom things are not.
> On 1 Feb 2016, at 11:17, Balaram Makam via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> This revision was automatically updated to reflect the committed changes.
> Closed by commit rL259387: AArch64: Implement missed conditional compare sequences. (authored by bmakam).
> Changed prior to commit:
> rL LLVM
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
More information about the llvm-commits