[PATCH] D36964: [AArch64] ISel legalization debug messages. NFCI.

[Alex Bradbury via Phabricator via llvm-commits]

asb added a comment.

In https://reviews.llvm.org/D36964#848739, @SjoerdMeijer wrote:

> Hi Alex, thanks for the feedback. The difficulty here is finding the right balance of adding (good) debug messages, and not cluttering the code too much. I think what you are suggesting is what I had in my first revision: that had debug messages in the different AArch64 lowering functions. Then Sam's suggestion was to move some of these messages to SelectionDAG (which we did in https://reviews.llvm.org/D36984), which has 2 advantages: all targets benefit, and it does not clutter up the code. Also, I think this is quite difficult to get completely right the first time. So I think we will have to see what works and what is good and I will be adding more stuff as I go along.  There's a lot of room for improvement here, but I already find these extra debug messages quite useful...


I definitely agree that this is something that can develop in time. I think splitting out some of the debug printing to the target-independent SelectionDAG code was a good move too. I suppose my comment only really applied to isLegalArithImmed. As you suggest, the other `is*` functions are called from outside AArch64ISelLowering anyway, and in the case of `isFPImmLegal` there's benefit in exposing details of that decision process. Anyway, I was interested if you'd thought about hoisting some of the debug messages to the caller, and the answer is obviously yes! This patch looks good to me, and I'm looking forward to monitoring how it this approach to ISel debugging develops.


https://reviews.llvm.org/D36964