[PATCH] D93853: [CodeGen] recognize select form of and/ors when splitting branch conditions

Mon Dec 28 03:31:35 PST 2020

nikic added a comment.

This looks correct to me, as we're breaking the select up into two branches, which means the second condition will get short-circuited entirely. As far as I can see, the conditions don't get swapped or similar.

However, I think it's worthwhile to consider an alternative here: We could simply fold select -> and/or in CGP, and forget about this as far as CodeGen is concerned. This is not technically correct, because SDAG also has a poison concept, but it's kind of a moot point, as DAGCombiner will end up doing the transform anyway: https://github.com/llvm/llvm-project/blob/a485a59d2172daaee1d5e734da54fbb243f7d54c/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp#L9197-L9220 That's something we could revisit if we ever wanted to drop the corresponding DAGCombiner fold (but I believe our current stance is to ignore such issues in the SDAG layer, as poison reasoning is used much less in this area and we've not seen any practical poison-related miscompiles in SDAG).

================
Comment at: llvm/lib/CodeGen/CodeGenPrepare.cpp:7868
     if (!match(Cond1, m_CombineOr(m_Cmp(), m_BinOp())) ||
-        !match(Cond2, m_CombineOr(m_Cmp(), m_BinOp()))   )
+        !match(Cond2, m_CombineOr(m_Cmp(), m_BinOp())))
       continue;
----------------
I think this will make the optimization not work for a chain of logical and/or, as this restricts to m_BinOp.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93853/new/

https://reviews.llvm.org/D93853