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

Viktor Kutuzov vkutuzov at accesssoftek.com
Mon Nov 16 18:05:03 PST 2009


Please find the attached patch.
Does this look like what you want to see?

I have moved the check for Darwin OS insode the getArchNameForAssembler which makes it OS and Vendor independent. This is what we wanted from the beginning, aren't we?
And added missed ppc64. Thanks for catching this.

Thanks,
Viktor.

________________________________________
From: Rafael Espindola [espindola at google.com]
Sent: Saturday, November 14, 2009 7:21 PM
To: Viktor Kutuzov
Cc: Commit Messages and Patches for LLVM
Subject: Re: [llvm-commits] [PATCH] LTO code generator options

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: llvm-lto-codegen-target-triple-asm.diff
Type: application/octet-stream
Size: 4772 bytes
Desc: llvm-lto-codegen-target-triple-asm.diff
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20091116/3f670d7a/attachment.obj>


More information about the llvm-commits mailing list