rengolin added a comment.
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.
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.
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.
If the function returns a string with the CPU name, and the CPU name is
"native", than logic dictates that the return value should be
std::string("native");
If the function wants to add additional logic to grab the native CPU name and
return that, it's extra. If it can't find the CPU name, it should return
"native", not "generic" or "cyclone".
Makes sense?
cheers,
--renato
http://reviews.llvm.org/D14471
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
