[PATCH] D42737: [LegalizeDAG] Truncate condition operand of ISD::SELECT

[Eugene Leviant via Phabricator via llvm-commits]

evgeny777 added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp:368
 
+  // Truncate the condition if needed
+  auto BoolVT = getSetCCResultType(CondVT);
----------------
delena wrote:
> evgeny777 wrote:
> > delena wrote:
> > > Can the condition be wider than getSetCCResultType?
> > Yes, if this wider type is legal for target architecture. However see @efriedma comment above.
> > 
> > If boolean type is not legal then you either crash when trying to expand condition or when trying to promote it (if type bit width is not power of 2). Here I chose to truncate it to size of getSetCCResultType to avoid promoting it back in `PromoteTargetBoolean`. 
> > 
> My question was incorrect. I wanted to ask whether the getSetCCResultType may be wider than Cond. Do you need to extend the Cond in this case?
I don't think so. `PromoteIntOp_SELECT` will do this for me. It calls `PromoteTargetBoolean` which in turn zero extends condition to the size of `getSetCCResultType`


================
Comment at: test/CodeGen/AArch64/expand-select.ll:4
+; Check that we don't crash
+; RUN: llc -mtriple=aarch64-unknown-linux-gnu -O3 %s -o -
+
----------------
delena wrote:
> evgeny777 wrote:
> > delena wrote:
> > > I think you should put here regular CHECKs, like in all other llc tests.
> > Any suggestion on what should be checked? Probably debug output from `-debug-only=selectiondag,isel`?
> just use utils/update_llc_test_checks.py to generate "CHECKS":
> 
> utils/update_llc_test_checks.py --llc-binary=<path>/llc test/CodeGen/AArch64/expand-select.ll
Thanks, I'll give it a try.


https://reviews.llvm.org/D42737