[llvm-commits] [PATCH] LTO code generator options

Rafael Espindola espindola at google.com
Sat Nov 14 19:21:47 PST 2009


2009/11/12 Viktor Kutuzov <vkutuzov at accesssoftek.com>:
> Thanks, Rafael.
>
> I have split this patch to a set of smaller ones. Hope this will make it easier to review.
> Please find attached 2 patches:
>
> * one adds to the Triple class
>   - a new getAssemblerArchName method, and
>   - assignment operators for the std::string and StringRef types;
>
> * the other one adds to the SubtargetFeature class
>   - a new getDefaultSubTargetTripleFeatures method (code moved from the LTOModule class),
>   - AddFeatures methods, and
>   - hasFeature method.
>
> I'll send the patch that use these changes after these 2 patches will be submitted.

I actually find it easier to review code when it is next to its use. Examples

*) I am not sure that including a llvm::Triple = std::string is the
best design. Without its use it is hard to tell.
*) Looking only at getArchNameForDarwinArchTriplePart it is hard to
check that it covers all cases.

The attached patch was extracted from your previous one. It includes
only the -arch option refactoring bits. I find it easier to read. For
example, I now noticed that you forgot to handle ppc64 :-)

I also adjusted it a bit for the coding style:

*) Try not to nest too much, so return null early if vendor != apple
*) Don't include an else after a return

Would you mind completing just this part first?

> Viktor.

Thanks,
-- 
Rafael Ávila de Espíndola
-------------- next part --------------
A non-text attachment was scrubbed...
Name: arch.patch
Type: text/x-diff
Size: 4746 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20091114/45469ee8/attachment.patch>


More information about the llvm-commits mailing list