[PATCH] D50179: [AArch64][ARM] Context sensitive meaning of option "crypto"

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added a comment.

Hi Eli, thanks for the feedback.

> Yes, this logic should be in TargetParser, not here. Trying to rewrite the target features afterwards is messy at best. (Actually, the target feature list generated by TargetParser probably shouldn't contain the string "crypto" at all.)

I appreciate there is room for improvement here, which is an understatement! :) I probably should have mentioned earlier that my colleague is working on targetparser and options, and he will send the proposal in the form of an RFC to the dev list soon. Very briefly, the proposal will elaborate on how we want to capture/enforce architecture extension dependencies (I believe thus also disallow architecturally invalid combinations), imply options, and e.g. warn on redundant options.

I want to move the crypto logic to this new framework as soon it is there. Thus, for the time being, this is a stopgap to demonstrate what we want to achieve (with crypto), and also quite importantly, we have something that works today. But again, I fully agree that the current implementation is far from ideal, but hopefully with these explanations is somewhat acceptable.


https://reviews.llvm.org/D50179