[PATCH] D17433: [ARM] fix initialization of PredictableSelectIsExpensive; NFC

Renato Golin via llvm-commits llvm-commits at lists.llvm.org
Sun Feb 21 11:22:00 PST 2016


rengolin added a comment.

In http://reviews.llvm.org/D17433#358148, @spatel wrote:

> So isLikeA9() is a *subset* of isOutOfOrder(). Therefore, the reason I said that this change would not be 'no functional change' is because A12 and A17 would now have 'PredictableSelectIsExpensive'. If A12 and A17 are in fact OoO designs, this is probably what we want.


A12 == A17, and both are OoO. So, isLikeA9, in this context, is the same as isOoO if you assume whoever created that method didn't know about A12/A17 (which I think it's true). Also, the issue here is to predict the impact of selects, which is mostly problematic on OoO cores, which is what the isLikeA9 was trying (and failing) to map.

> This is the problem with having codegen predicates based on micro-arch models rather than features ("isLikeA9"); they tend to drift in meaning as newer designs are added. I've been trying to remove all of those in the x86 backend.


But you're absolutely right that this is a horrible way of modelling features, and we should really move away, and hopefully delete that method ASAP.

Since the scheduler model is what matters here, the change is exactly what we need here. Also, once we have a scheduler for A15 and A17, this code will still be correct, so it's also future-proof.

Junmo, please remove the NFC comment from the commit message. Also, do you have access to A9/A15 cores?

cheers,
--renato


http://reviews.llvm.org/D17433





More information about the llvm-commits mailing list