A question about the widely used "xxx | true" inside llvm

This is better suited to llvm-dev, so I’m directing it there.

The language requires both sides of the | need to be evaluated. The compiler can’t remove the function call if it has side effects.

Shoaib Meenai via llvm-dev <llvm-dev@lists.llvm.org> writes:

This is better suited to llvm-dev, so I'm directing it there.

From: llvm-commits <llvm-commits-bounces@lists.llvm.org> on behalf of
Qing Shan Zhang via llvm-commits <llvm-commits@lists.llvm.org>
Reply-To: Qing Shan Zhang <qshanz@cn.ibm.com>
Date: Sunday, August 19, 2018 at 10:10 PM
To: "llvm-commits@lists.llvm.org" <llvm-commits@lists.llvm.org>
Subject: A question about the widely used "xxx | true" inside llvm

Hi, guys,

I notice that, we are widely using the statement "xxx | true" in llvm,
such as "return SimplifyCFG(BB) | true;". I have no idea if there is
any downside to use the logic operator "||" here. Instead, I indeed
see the potential problem to use bitwise operator "|". Because the
bitwise operator do not short-circuit and the language std didn't
specify the evaluation order for its lhs and rhs expression. That
means, compiler is free to optimize this statement to "return true".

I think "widely using" is overstating the issue. This seems to be
happening in a lot of places in SimplifyCFG though, and from looking at
the code and the history it's pretty clear to me that the uses of the
bitwise or are either stylistic or unintentional. I've gone ahead and
changed all of those to use || for clarity in r340153.

In shorts, IMO, it is legal to do the following optimization:
bool bar() {
  return foo() | true;
}
-->
bool bar() {
  return true; // the call to foo() is missing.
}

This is only legal if the compiler can prove that foo() has no side
effects, which isn't true in this case. Using bitwise or here is
certainly confusing, but there isn't a correctness issue.

Correct. If you are somehow really bothered by this, we could also use:

bool bar() {
return foo(), true;
}

-Chris

I believe Justin Bogner went ahead and changed all the lines in SimplifyCFG to || in r340153.

~Craig