Re: [PATCH] D16139: [MIPS] initFeatureMap() to handle empty string argument

2016-03-23 Thread Daniel Sanders via cfe-commits
dsanders added a comment. > > > b) CPUs are not subtarget features (or they shouldn't be), they're CPUs > > > that contain features. They may be generic names for ISAs as well, but > > > probably best to keep them separate. > > > > I agree, we have two separate concepts that happen to use t

Re: [PATCH] D16139: [MIPS] initFeatureMap() to handle empty string argument

2016-03-22 Thread Eric Christopher via cfe-commits
iews.llvm.org; Bhushan > Attarde; Vasileios Kalintiris; Daniel Sanders > > > Cc: Sagar Thakur; Nitesh Jain; Mohit Bhakkad; Jaydeep Patil; > cfe-commits@lists.llvm.org > > > Subject: Re: [PATCH] D16139: [MIPS] initFeatureMap() to handle empty > string argument > > >

RE: [PATCH] D16139: [MIPS] initFeatureMap() to handle empty string argument

2016-03-09 Thread Daniel Sanders via cfe-commits
> > cfe-commits@lists.llvm.org > > Subject: Re: [PATCH] D16139: [MIPS] initFeatureMap() to handle empty string > > argument > > > > On Sat, Mar 5, 2016 at 6:16 AM Daniel Sanders > > mailto:daniel.sand...@imgtec.com>> wrote: > > dsanders added

Re: [PATCH] D16139: [MIPS] initFeatureMap() to handle empty string argument

2016-03-08 Thread Eric Christopher via cfe-commits
On Sat, Mar 5, 2016 at 6:16 AM Daniel Sanders wrote: > dsanders added a comment. > > In http://reviews.llvm.org/D16139#368217, @echristo wrote: > > > This seems wrong. You should fix setCPU instead or set a default CPU. > > > We already set a default CPU in the constructor (e.g. > Mips32TargetInf

Re: [PATCH] D16139: [MIPS] initFeatureMap() to handle empty string argument

2016-03-05 Thread Daniel Sanders via cfe-commits
dsanders added a comment. In http://reviews.llvm.org/D16139#368217, @echristo wrote: > This seems wrong. You should fix setCPU instead or set a default CPU. We already set a default CPU in the constructor (e.g. Mips32TargetInfoBase::Mips32TargetInfoBase() provides "mips32r2"). It's the CPU ar

Re: [PATCH] D16139: [MIPS] initFeatureMap() to handle empty string argument

2016-03-04 Thread Eric Christopher via cfe-commits
echristo added a subscriber: echristo. echristo added a comment. This seems wrong. You should fix setCPU instead or set a default CPU. Repository: rL LLVM http://reviews.llvm.org/D16139 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http

Re: [PATCH] D16139: [MIPS] initFeatureMap() to handle empty string argument

2016-03-03 Thread Daniel Sanders via cfe-commits
dsanders accepted this revision. dsanders added a comment. Thanks. LGTM Repository: rL LLVM http://reviews.llvm.org/D16139 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D16139: [MIPS] initFeatureMap() to handle empty string argument

2016-02-22 Thread Bhushan Attarde via cfe-commits
bhushan added a comment. > I'm guessing it's something to do with the 'Features[CPU] = true' line. Yes. `Features[CPU] = true` enables a feature given by CPU string. (This gets translated into "+CPU"). When CPU string is empty, this gets translated into "+" (i.e. with empty feature name). Thi

Re: [PATCH] D16139: [MIPS] initFeatureMap() to handle empty string argument

2016-02-19 Thread Daniel Sanders via cfe-commits
dsanders added a comment. Can you explain what the problem was and why this change is needed? I'm guessing it's something to do with the 'Features[CPU] = true' line. Repository: rL LLVM http://reviews.llvm.org/D16139 ___ cfe-commits mailing list

Re: [PATCH] D16139: [MIPS] initFeatureMap() to handle empty string argument

2016-02-18 Thread Bhushan Attarde via cfe-commits
bhushan added a comment. This was observed during expression evaluation in LLDB, where LLDB does not explicitly set `clang::TargetOptions::CPU` hence it remains empty. Repository: rL LLVM http://reviews.llvm.org/D16139 ___ cfe-commits mailing li

Re: [PATCH] D16139: [MIPS] initFeatureMap() to handle empty string argument

2016-02-17 Thread Vasileios Kalintiris via cfe-commits
vkalintiris accepted this revision. vkalintiris added a comment. This revision is now accepted and ready to land. LGTM. However, I'm curious to learn which case triggered this behaviour. Repository: rL LLVM http://reviews.llvm.org/D16139 ___ cfe-