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

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

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

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

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

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

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

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

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

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

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

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

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

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