[PATCH] D35522: Move Runtime libcall definitions to a .def file

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: lib/CodeGen/TargetLoweringBase.cpp:100-110
   if (TT.isGNUEnvironment()) {
     Names[RTLIB::SINCOS_F32] = "sincosf";
     Names[RTLIB::SINCOS_F64] = "sincos";
     Names[RTLIB::SINCOS_F80] = "sincosl";
     Names[RTLIB::SINCOS_F128] = "sincosl";
     Names[RTLIB::SINCOS_PPCF128] = "sincosl";
   }
----------------
dschuff wrote:
> dschuff wrote:
> > dblaikie wrote:
> > > Any reason these two are implemented differently (one has null in the table and initializer the values conditionally - the other has a value in the table and conditionally nulls it out here)?
> > The idea was sort of that the default value in the table is the "normal" or most common value, and the exceptional cases are handled here.
> Another somewhat-related note, there ended up being a slight bit of ugliness (see line 883 of D35592) related to using nullptr here instead of e.g. an empty string. But that maybe is still better than using empty string instead of nullptr here?
I'd /probably/ keep it one way or the other rather than picking/judging which case is more common/'normal'. But no big deal either way/whatever you prefer.


================
Comment at: lib/CodeGen/TargetLoweringBase.cpp:100-110
   if (TT.isGNUEnvironment()) {
     Names[RTLIB::SINCOS_F32] = "sincosf";
     Names[RTLIB::SINCOS_F64] = "sincos";
     Names[RTLIB::SINCOS_F80] = "sincosl";
     Names[RTLIB::SINCOS_F128] = "sincosl";
     Names[RTLIB::SINCOS_PPCF128] = "sincosl";
   }
----------------
dblaikie wrote:
> dschuff wrote:
> > dschuff wrote:
> > > dblaikie wrote:
> > > > Any reason these two are implemented differently (one has null in the table and initializer the values conditionally - the other has a value in the table and conditionally nulls it out here)?
> > > The idea was sort of that the default value in the table is the "normal" or most common value, and the exceptional cases are handled here.
> > Another somewhat-related note, there ended up being a slight bit of ugliness (see line 883 of D35592) related to using nullptr here instead of e.g. an empty string. But that maybe is still better than using empty string instead of nullptr here?
> I'd /probably/ keep it one way or the other rather than picking/judging which case is more common/'normal'. But no big deal either way/whatever you prefer.
Using empty string seems like a fine idea & seems worthwhile given how it'd simplify the next one.


https://reviews.llvm.org/D35522