[PATCH] D86871: [SelectionDAG] Let NSW/NUW flags be cleared by default in call to getNode().

Ulrich Weigand via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Sep 4 03:16:32 PDT 2020


uweigand added a comment.

> I may be missing something, but as long as the memoization does a true intersection of the flags per this patch, there should not necessarily be anything broken, or?

The problem occurs when setFlags is done **after** memoization was already performed.   For example, this code in LegalizeVectorTypes.cpp:

  SDNode *ScalarNode = DAG.getNode(
      N->getOpcode(), DL, ScalarVTs, ScalarLHS, ScalarRHS).getNode();
  ScalarNode->setFlags(N->getFlags());

This creates a new scalar node with no flags.  Now, it might happen that this same node already exists in the DAG, in a place where no flags are valid.  Memoization returns this existing node; intersection doesn't change anything since there already were no flags set.  But **after** that memoized node is returned, the explicit setFlags call now simply overwrites the flags.  If some flag bits are set here, these will now apply to the original node, where they may be incorrect.

Fortunately, it looks like most such cases in SelectionDAG can be fixed by simply providing the desired flags with get getNode call itself and removing the separate setFlags call.


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

https://reviews.llvm.org/D86871



More information about the llvm-commits mailing list