[PATCH] D35635: Optimize {s,u}{add,sub}.with.overflow on ARM

Joel Galenson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Oct 18 13:38:22 PDT 2017


jgalenson added a comment.

> It doesn't quite work the way you want it to because we don't update the ordering when we add new nodes to the DAG.  But I haven't thought through how to compute the right order to visit without recomputing the entire topological order from scratch.

This does sound like it would solve the problem (and solve other problems as well).  It seems a bit out of scope for this commit, though.

> Alternatively, you could try DAGCombining ARMISD::BRCOND after legalization?

Good idea.  However, the ARMISD::BRCOND isn't combined until after saddo is lowered.

For reference, here's the sequence of events I'm seeing in this one example:

brcond is legalized to br_cc
saddo is legalized
br_cc is legalized to ARMISD::BRCOND
ARMISB::BRCOND is legalized
everything is combined

So combining ARMISD::BRCOND runs into the same problem as using br_cc; they're both too late.  Thus without changing the ordering of new DAG nodes (which seems a bit difficult), I don't see how I can do this efficiently without keeping the brcond case.


https://reviews.llvm.org/D35635





More information about the llvm-commits mailing list