[llvm] r178416 - [NVPTX] Add NVVMReflect pass to allow compile-time selection of

[Justin Holewinski]

On Sat, Mar 30, 2013 at 11:58 AM, Sean Silva <silvas at purdue.edu> wrote:

> Could you give a bit more background on the motivation for this? I wasn't
> able to find any discussion on llvmdev.
>

The only real motivation is compatibility with CUDA.  This pass is needed
for linking with certain CUDA libraries that rely on this to implement
selecting between code paths based on fast-math flags.  I would love to get
rid of this in the future, but it was created long before I had any say in
it. :)

The idea is that most users won't use it, but those that want to link to
certain CUDA libraries will (at least for now).  I just want to upstream it
to maximize compatibility between the open-source version and the
proprietary bits that ship with CUDA.


>
> +// This pass replaces occurences of __nvvm_reflect("string") with an
> +// integer based on -nvvm-reflect-list string=<int> option given to this
> pass.
>
> Please give an example of usage here.
>
> It also seems that in some places you say "0/1", but in this description
> you say "<int>". Is any int allowed? Please clarify.
>

Technically any int is allowed, but only 0/1 is actually used in practice
right now.  I'll update the comments.


>
> +    int reflectVal = 0; // The default value is 0
>
> Please put this information in the top-of-the-file comment.
>

Sure.


>
> +  for (unsigned i = 0, e = ReflectList.size(); i != e; ++i) {
> +    //    DEBUG(dbgs() << "Option : "  << ReflectList[i] << std::endl);
>
> +    reflectArg = reflectArg.substr(0, reflectArg.size() - 1);
> +    //    DEBUG(dbgs() << "Arg of _reflect : " << reflectArg <<
> std::endl);
>
> +  //std::map<std::string, int> VarMap;
>
> +  StringMap<int> VarMap;
>
> Please remove commented-out code.
>

Oops...


>
> +static std::vector<std::string>
> +Tokenize(const std::string &str, const std::string &delim) {
> +  std::vector<std::string> tokens;
>
> Is there a particular reason you aren't following the LLVM naming
> convention here and throughout?
>

Not really.  Internally, attempts to follow the LLVM style are lax.  This
is just a case I missed... :(

Thanks for the review!


>
> -- Sean Silva
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>


-- 

Thanks,

Justin Holewinski
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130330/d615d233/attachment.html>