[PATCH] D41689: [SCEVAA] Don't crash on pointers with no dominance relationship.

Hal Finkel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Jan 6 19:41:28 PST 2018

hfinkel added a comment.

> I'm not sure this is the right fix; it seems like I'm working around the underlying issue rather than actually solving it.

I agree. This seems like working around the underlying issue.

> Does it makes sense to call AliasAnalysis::alias() in the case where HasDominanceRelation returns false?

No, I don't think it does (for the reason you've specified). We only support aliasing queries between values with some dominance relation.

I couple of thoughts:

1. If we wanted to handle this somehow in SCEVAAResult::alias, can't we just put a dominance check on the values directly instead of trying to examine the SCEVs? Meaning:

  Instruction *IA = dyn_cast<Instruction>(LocA.Ptr),
                        *IB = dyn_cast<Instruction>(LocB.Ptr);
  if (IA && IB &&
       (DT->dominates(IA->getParent(), IB->getParent()) ||
        DT->dominates(IB->getParent(), IA->getParent())) &&
       "Queries between values without a dominance relationship is not supported");

(or returning a conservative answer instead of asserting)

2. I think that what AliasSetPrinter is doing is just not well defined. We should really fix it (although, frankly, I'm not sure how without increasing its asymptotic complexity by having it create a set for each block only with others with a dominance relationship, but maybe that's okay as AliasSetPrinter is a diagnostic tool).



More information about the llvm-commits mailing list