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 the same 
> strings. We should probably map them explicitly.

>  > 

>  Yes, and that's what you should do instead of this patch.


Just making the mapping explicit isn't enough by itself. We still have to 
resolve the empty string to the default CPU because initFeatureMap()'s CPU 
argument dodges the defaults that are normally handled in the constructor. I'll 
post patches soon (I've been away for a few days too) I just need to test a 
couple changes we're agreed on first and sort out some remaining IAS work.

> > > c) You should set the features based on the CPUs given to the function. 
> > > The typical way the cpu comes in, is via -target-cpu which comes via:

>  > >

>  > >   case llvm::Triple::mips:

>  > >   case llvm::Triple::mipsel:

>  > >   case llvm::Triple::mips64:

>  > >   case llvm::Triple::mips64el: {

>  > >     StringRef CPUName;

>  > >     StringRef ABIName;

>  > >     mips::getMipsCPUAndABI(Args, T, CPUName, ABIName);

>  > >     return CPUName;

>  > >   }

>  > >

>  > > for mips.

>  > >

>  > > Now if your triple is returning an empty string here you might have 
> gotten to where you are (I tried mips64r2-linux-gnu as the -target option). 
> Which is what typically happens down

>  > >  this path.

>  > 

>  > This usage is from the clang driver. On this path, getMipsCPUAndABI 
> ensures that the CPU is never empty.

> 

> I gave you a testcase that can prove otherwise in my earlier email.


I don't think it proves otherwise. The triple you quoted isn't a supported MIPS 
triple and LLVM will reject it. LLVM for MIPS doesn't accept CPU names in the 
first component of the triple and this particular one isn't known to gcc 
either. Can you give me the exact command you tried? I get this:

  $ bin/clang -target mips64r2-linux-gnu -o hello.s -S hello.c
  error: unknown target triple 'mips64r2--linux-gnu', please use -triple or 
-arch
  $ bin/llc -mtriple mips64r2-linux-gnu -o hello.s hello.bc
  bin/llc: : error: unable to get target for 'mips64r2--linux-gnu', see 
--version and --triple.

and if a triple that wasn't mips/mipsel/mips64/mips64el somehow got in to 
getMipsCPUAndABI() it would trigger an llvm_unreachable().

It's impossible to provide a known MIPS triple and end up with the empty string 
as the CPU name within the clang driver. The empty string is only known to 
occur from LLDB's usage.


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

Reply via email to