Re: [PATCH] D14471: [AArch64] Fix a crash in driver

ahatanak abandoned this revision. ahatanak added a comment. The crash I was trying to fix was fixed in https://reviews.llvm.org/D23643. https://reviews.llvm.org/D14471 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi

Re: [PATCH] D14471: [AArch64] Fix a crash in driver

ahatanak added a comment. Sorry for not replying for a long time. I'll get back to this soon. http://reviews.llvm.org/D14471 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D14471: [AArch64] Fix a crash in driver

rengolin added a comment. If this patch was abandoned, please "Abandon" it. If not, please implement it using the AArch64 TargetParser. http://reviews.llvm.org/D14471 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi

Re: [PATCH] D14471: [AArch64] Fix a crash in driver

rengolin added subscribers: labrinea, bsmith. rengolin added a comment. Hi, I think it's clear now to me that this strategy isn't going to work. What you need is to add support for AArch64 in the TargetParser and simplify the name matching in the same way we did for ARM. There really is no othe

Re: [PATCH] D14471: [AArch64] Fix a crash in driver

ahatanak updated this revision to Diff 46725. ahatanak added a comment. Sorry for taking so long to get back to this patch. I've updated the patch based on your review comments. I defined a new helper function getAArch64FeaturesFromCPU which gets the target features from the cpu name and checks

Re: [PATCH] D14471: [AArch64] Fix a crash in driver

rengolin added a comment. In http://reviews.llvm.org/D14471#288621, @ahatanak wrote: > In getAArch64TargetCPU, if it finds out the cpu name passed via -mtune or > -mcpu is "native", > > 1. Call llvm::sys::getHostCPUName to get the host CPU name. > 2. Check the host CPU name to see if it is a val

Re: [PATCH] D14471: [AArch64] Fix a crash in driver

ahatanak added a comment. > If you can get the CPU name, return it. If not, return "native". If you're on > AArch64 and you can't get the CPU name, that's a bug that needs fixing. If > it's always "generic", that's another piece of code that needs fixing. If > whatever getCPU function you use d

Re: [PATCH] D14471: [AArch64] Fix a crash in driver

rengolin added a comment. In http://reviews.llvm.org/D14471#287635, @ahatanak wrote: > 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 whic

Re: [PATCH] D14471: [AArch64] Fix a crash in driver

On 11/11/15 5:01 PM, Akira Hatanaka via cfe-commits wrote: 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 th

Re: [PATCH] D14471: [AArch64] Fix a crash in driver

ahatanak added a comment. Sorry, there were mistakes in my comments. What I meant to say is that changing getAArch64TargetCPU to return "native" would break the case where clang is being run on an aarch64 host. The current code in trunk will get the host cpu name (which I believe is currently a

Re: [PATCH] D14471: [AArch64] Fix a crash in driver

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

Re: [PATCH] D14471: [AArch64] Fix a crash in driver

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.

Re: [PATCH] D14471: [AArch64] Fix a crash in driver

ahatanak added a comment. I think I understand some of your concerns, but I'm not sure I fully understand what you are suggesting. 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

Re: [PATCH] D14471: [AArch64] Fix a crash in driver

rengolin added a comment. In http://reviews.llvm.org/D14471#285492, @ahatanak wrote: > Which two calls? Do you mean getAArch64TargetCPU and > getAArch64ArchFeaturesFromMcpu? No, the two getAArch64TargetCPU calls, but they don't even get called twice, and the value is write-only. Ignore that.

Re: [PATCH] D14471: [AArch64] Fix a crash in driver

ahatanak added a comment. Hi Renato In http://reviews.llvm.org/D14471#285138, @rengolin wrote: > I'm uncomfortable with this change, since it introduces a dependency between > the two calls, and that's fragile. Also, the nullptr fiddling is not a good > design overall. > > If there is a depend

Re: [PATCH] D14471: [AArch64] Fix a crash in driver

rengolin added a comment. Hi Akira, I'm uncomfortable with this change, since it introduces a dependency between the two calls, and that's fragile. Also, the nullptr fiddling is not a good design overall. If there is a dependency, I suggest you encode it directly into getAArch64TargetFeatures

Re: [PATCH] D14471: [AArch64] Fix a crash in driver

ahatanak added a comment. Send the patch to cfe-commits. http://reviews.llvm.org/D14471 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits