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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to