[PATCH] D53190: ARM: avoid infinite combining loop

Kristof Beyls via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 25 01:24:38 PDT 2018


kristof.beyls added a comment.

I guess you're mainly asking review on this to get opinions on the approach to introduce new pseudo instructions just to avoid cycles during DAG combining and/or lowering?
>From the patch, I furthermore guess that introducing a new pseudo leads to quite a bit of code duplication/violations of the "don't-repeat-yourself" principle in the instruction matching patterns?

With that, I wonder:

- It seems unlikely this is the only place where a problem with cycles during DAG combining/lowering pops up.
  - If it is the only place: what is so exceptional here that this really is the only place?
  - If it is not: what are the other approaches already taken elsewhere to avoid cycles? Why are alternative approaches not applicable here (or why are they an even worse solution)? Is there actually some guidance written up somewhere on which approaches exist and which to take to avoid cycles?
- Instead of producing a whole new pseudo, I guess an alternative would be to introduce a way to flag on ISD nodes "do not combine this node any further" as a more generic mechanism. I guess for that to work, some bit somewhere on the ISD node would be needed to store that info. I wonder if you considered such an approach too? I guess it has the advantage that it doesn't require violating the DRY principle in the instruction patterns?


Repository:
  rL LLVM

https://reviews.llvm.org/D53190





More information about the llvm-commits mailing list