[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

Hi Marcello,

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:
  if (!DAG.getTargetLoweringInfo().isTypeLegal(VT))
    return SDValue();

-----Original Message-----
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:
>  http://reviews.llvm.org/D16291?vs=46546&id=46561#toc
> Repository:
>  rL LLVM
> http://reviews.llvm.org/D16291
> Files:
>  llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
>  llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.cpp
>  llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.h
>  llvm/trunk/test/CodeGen/AArch64/arm64-ccmp.ll
> <D16291.46561.patch>_______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits

More information about the llvm-commits mailing list