ahatanak added a comment.

In http://reviews.llvm.org/D14471#287412, @rengolin wrote:

> In http://reviews.llvm.org/D14471#286380, @ahatanak wrote:
>
> > I think I can use macro __aarch64__ to have getAArch64TargetCPU return 
> > "native" when the compiler is not run on an AArch64 platform, but it 
> > doesn't sound like that was what you had in mind?
>
>
> Not at all. :)
>
> It seems like a simple thing really, but it piles up pretty quickly.
>
> The command line:
>
>   $ clang -arch arm64 -mtune=native -c hello.c
>   
>
> doesn't make sense on an x86 machine at all. "native" means it should 
> understand the host as an ARM64 hardware, which it's not. This should bail on 
> error.


I agree and that is what I'm trying to do. It should error out with something 
like "native is not supported".

> You haven't provided any tests (you should really, what should work and what 
> should return errors), so I can't tell just by looking at the code what you 
> expect the new behaviour to be. I'm assuming you still want things to break, 
> but you want your error message to have a "native" in it instead of crashing 
> the compiler.


That's correct. The error message I'm looking for is something like the one I 
wrote in the summary:

error: the clang compiler does not support 'native'

I didn't include a test case because I didn't know how to write a test that 
passes on an aarch64 host and fails on anything else. Do you know of any test 
cases in trunk that pass or fail depending on which host it is running on?

> Back to the code...

> 

> Passing the pointer to a boolean is leaking logic from getAArch64TargetCPU 
> out. If you know inside that the CPU is "native", why not just return that 
> instead? Would that break other things? If so, these other things should be 
> fixed. Fudging the parameters and keeping every other caller in the dark 
> (because you're overriding the bool pointer) is certainly not the way we want 
> to go, because that would mean two different behaviours depending on how you 
> call the function.


Wouldn't it break on an aarch64 host? With "-mtune=native", the current code in 
trunk will get the host cpu name (which I believe is currently always "generic" 
for aarch64). But if I apply this patch, it will error out because "native" is 
not a valid cpu name.


http://reviews.llvm.org/D14471



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

Reply via email to