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

[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 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