[PATCH] D10969: Replace Triple with a new TargetTuple in MCTargetDesc/* and related. NFC.

Daniel Sanders via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 15 11:36:47 PDT 2015


> BTW I do agree that it's completely unreasonable how long it's taken me on this. I do truly apologize, ...

Thanks. I appreciate that.

________________________________
From: Eric Christopher [echristo at gmail.com]
Sent: 15 September 2015 17:27
To: Daniel Sanders; reviews+D10969+public+5f3f4828b5695b54 at reviews.llvm.org; renato.golin at linaro.org
Cc: jyknight at google.com; dschuff at google.com; Matthew.Arsenault at amd.com; stanislav.mekhanoshin at amd.com; danalbert at google.com; srhines at google.com; javed.absar at arm.com; emaste at freebsd.org; jholewinski at nvidia.com; tberghammer at google.com; ted.woodward at codeaurora.org; jfb at chromium.org; llvm-commits at lists.llvm.org
Subject: Re: [PATCH] D10969: Replace Triple with a new TargetTuple in MCTargetDesc/* and related. NFC.

BTW I do agree that it's completely unreasonable how long it's taken me on this. I do truly apologize, it was only when looking at the recent patch dealing with ABIInfo that you approved that I realized what you're trying to deal with in this and how it's interacting with everything else. It's absolutely needed so let's discuss and move this forward.

-eric

On Tue, Sep 15, 2015 at 9:12 AM Eric Christopher <echristo at gmail.com<mailto:echristo at gmail.com>> wrote:
I've replied in a different thread with what I see is the design that needs to happen here.

-eric

On Tue, Sep 15, 2015 at 9:07 AM Daniel Sanders <Daniel.Sanders at imgtec.com<mailto:Daniel.Sanders at imgtec.com>> wrote:
Sorry, I accidentally hit Ctrl+Enter. My next email will contain a reply :-)
________________________________
From: Daniel Sanders
Sent: 15 September 2015 17:05
To: Eric Christopher; reviews+D10969+public+5f3f4828b5695b54 at reviews.llvm.org<mailto:reviews%2BD10969%2Bpublic%2B5f3f4828b5695b54 at reviews.llvm.org>; renato.golin at linaro.org<mailto:renato.golin at linaro.org>

Cc: jyknight at google.com<mailto:jyknight at google.com>; dschuff at google.com<mailto:dschuff at google.com>; Matthew.Arsenault at amd.com<mailto:Matthew.Arsenault at amd.com>; stanislav.mekhanoshin at amd.com<mailto:stanislav.mekhanoshin at amd.com>; danalbert at google.com<mailto:danalbert at google.com>; srhines at google.com<mailto:srhines at google.com>; javed.absar at arm.com<mailto:javed.absar at arm.com>; emaste at freebsd.org<mailto:emaste at freebsd.org>; jholewinski at nvidia.com<mailto:jholewinski at nvidia.com>; tberghammer at google.com<mailto:tberghammer at google.com>; ted.woodward at codeaurora.org<mailto:ted.woodward at codeaurora.org>; jfb at chromium.org<mailto:jfb at chromium.org>; llvm-commits at lists.llvm.org<mailto:llvm-commits at lists.llvm.org>
Subject: RE: [PATCH] D10969: Replace Triple with a new TargetTuple in MCTargetDesc/* and related. NFC.

> I started a reply yesterday actually and I'm sorry I didn't finish it.

> Briefly I don't think this is the right direction and I don't think you've explained why this is the right direction.

> The serialization that you have planned is insufficient and doesn't take into account portability

> I don't like the idea of using global metadata for it and you haven't explained why this is acceptable

________________________________
From: Eric Christopher [echristo at gmail.com<mailto:echristo at gmail.com>]
Sent: 15 September 2015 16:56
To: Daniel Sanders; reviews+D10969+public+5f3f4828b5695b54 at reviews.llvm.org<mailto:reviews%2BD10969%2Bpublic%2B5f3f4828b5695b54 at reviews.llvm.org>; renato.golin at linaro.org<mailto:renato.golin at linaro.org>
Cc: jyknight at google.com<mailto:jyknight at google.com>; dschuff at google.com<mailto:dschuff at google.com>; Matthew.Arsenault at amd.com<mailto:Matthew.Arsenault at amd.com>; stanislav.mekhanoshin at amd.com<mailto:stanislav.mekhanoshin at amd.com>; danalbert at google.com<mailto:danalbert at google.com>; srhines at google.com<mailto:srhines at google.com>; javed.absar at arm.com<mailto:javed.absar at arm.com>; emaste at freebsd.org<mailto:emaste at freebsd.org>; jholewinski at nvidia.com<mailto:jholewinski at nvidia.com>; tberghammer at google.com<mailto:tberghammer at google.com>; ted.woodward at codeaurora.org<mailto:ted.woodward at codeaurora.org>; jfb at chromium.org<mailto:jfb at chromium.org>; llvm-commits at lists.llvm.org<mailto:llvm-commits at lists.llvm.org>
Subject: Re: [PATCH] D10969: Replace Triple with a new TargetTuple in MCTargetDesc/* and related. NFC.


I started a reply yesterday actually and I'm sorry I didn't finish it. Briefly I don't think this is the right direction and I don't think you've explained why this is the right direction. The serialization that you have planned is insufficient and doesn't take into account portability, I don't like the idea of using global metadata for it and you haven't explained why this is acceptable. I actually was spending a non-trivial amount of time trying to design a way for this to work along side the assembler and main compiler (which I don't think you've bothered to try to do)

Please revert this immediately.

-eric


On Tue, Sep 15, 2015, 6:29 AM Daniel Sanders <Daniel.Sanders at imgtec.com<mailto:Daniel.Sanders at imgtec.com>> wrote:
(apologies if this double-posts, it seems Phabricator just died)

I've committed this (along with the trivial clang patch) in r247683 on the grounds that I have Renato's LGTM and I don't have any pending review comments. It doesn't seem reasonable to continue to wait for Eric with no indication of when he might be able to take a look at it. For the record, I've been waiting since July and my request last week for an indication as to when he might find time has not been replied to. Of course, post-commit review comments are still welcome and I'm happy to follow up on them.

> -----Original Message-----
> From: Daniel Sanders
> Sent: 15 September 2015 14:20
> To: Daniel Sanders; renato.golin at linaro.org<mailto:renato.golin at linaro.org>
> Cc: jyknight at google.com<mailto:jyknight at google.com>; dschuff at google.com<mailto:dschuff at google.com>;
> Matthew.Arsenault at amd.com<mailto:Matthew.Arsenault at amd.com>; stanislav.mekhanoshin at amd.com<mailto:stanislav.mekhanoshin at amd.com>;
> danalbert at google.com<mailto:danalbert at google.com>; srhines at google.com<mailto:srhines at google.com>; javed.absar at arm.com<mailto:javed.absar at arm.com>;
> echristo at gmail.com<mailto:echristo at gmail.com>; emaste at freebsd.org<mailto:emaste at freebsd.org>; jholewinski at nvidia.com<mailto:jholewinski at nvidia.com>;
> tberghammer at google.com<mailto:tberghammer at google.com>; ted.woodward at codeaurora.org<mailto:ted.woodward at codeaurora.org>;
> jfb at chromium.org<mailto:jfb at chromium.org>; llvm-commits at lists.llvm.org<mailto:llvm-commits at lists.llvm.org>
> Subject: Re: [PATCH] D10969: Replace Triple with a new TargetTuple in
> MCTargetDesc/* and related. NFC.
>
> This revision was automatically updated to reflect the committed changes.
> Closed by commit rL247683: Replace Triple with a new TargetTuple in
> MCTargetDesc/* and related. NFC. (authored by dsanders).
>
> Changed prior to commit:
>   http://reviews.llvm.org/D10969?vs=34223&id=34801#toc
>
> Repository:
>   rL LLVM
>
> http://reviews.llvm.org/D10969
>
> Files:
>   cfe/trunk/lib/Parse/ParseStmtAsm.cpp
>   cfe/trunk/tools/driver/cc1as_main.cpp
>   llvm/trunk/include/llvm/ADT/TargetTuple.h
>   llvm/trunk/include/llvm/MC/MCELFObjectWriter.h
>   llvm/trunk/include/llvm/MC/MCSubtargetInfo.h
>   llvm/trunk/include/llvm/Support/TargetRegistry.h
>   llvm/trunk/include/llvm/Target/TargetMachine.h
>   llvm/trunk/include/llvm/Target/TargetSubtargetInfo.h
>   llvm/trunk/lib/CodeGen/LLVMTargetMachine.cpp
>   llvm/trunk/lib/MC/MCDisassembler/Disassembler.cpp
>   llvm/trunk/lib/MC/MCDisassembler/MCExternalSymbolizer.cpp
>   llvm/trunk/lib/MC/MCDisassembler/MCRelocationInfo.cpp
>   llvm/trunk/lib/MC/MCSubtargetInfo.cpp
>   llvm/trunk/lib/Support/CMakeLists.txt
>   llvm/trunk/lib/Support/TargetTuple.cpp
>   llvm/trunk/lib/Target/AArch64/AArch64Subtarget.cpp
>   llvm/trunk/lib/Target/AArch64/AArch64Subtarget.h
>   llvm/trunk/lib/Target/AArch64/Disassembler/AArch64Disassembler.cpp
>   llvm/trunk/lib/Target/AArch64/MCTargetDesc/AArch64AsmBackend.cpp
>   llvm/trunk/lib/Target/AArch64/MCTargetDesc/AArch64ELFStreamer.cpp
>   llvm/trunk/lib/Target/AArch64/MCTargetDesc/AArch64MCAsmInfo.cpp
>   llvm/trunk/lib/Target/AArch64/MCTargetDesc/AArch64MCAsmInfo.h
>   llvm/trunk/lib/Target/AArch64/MCTargetDesc/AArch64MCTargetDesc.cpp
>   llvm/trunk/lib/Target/AArch64/MCTargetDesc/AArch64MCTargetDesc.h
>   llvm/trunk/lib/Target/AMDGPU/AMDGPUSubtarget.cpp
>   llvm/trunk/lib/Target/AMDGPU/MCTargetDesc/AMDGPUAsmBackend.cpp
>   llvm/trunk/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCAsmInfo.cpp
>   llvm/trunk/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCAsmInfo.h
>
> llvm/trunk/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCTargetDesc.cpp
>   llvm/trunk/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCTargetDesc.h
>   llvm/trunk/lib/Target/ARM/ARMAsmPrinter.cpp
>   llvm/trunk/lib/Target/ARM/ARMSubtarget.cpp
>   llvm/trunk/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
>   llvm/trunk/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.h
>   llvm/trunk/lib/Target/ARM/MCTargetDesc/ARMAsmBackendDarwin.h
>   llvm/trunk/lib/Target/ARM/MCTargetDesc/ARMAsmBackendELF.h
>   llvm/trunk/lib/Target/ARM/MCTargetDesc/ARMAsmBackendWinCOFF.h
>   llvm/trunk/lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp
>   llvm/trunk/lib/Target/ARM/MCTargetDesc/ARMMCAsmInfo.cpp
>   llvm/trunk/lib/Target/ARM/MCTargetDesc/ARMMCAsmInfo.h
>   llvm/trunk/lib/Target/ARM/MCTargetDesc/ARMMCCodeEmitter.cpp
>   llvm/trunk/lib/Target/ARM/MCTargetDesc/ARMMCTargetDesc.cpp
>   llvm/trunk/lib/Target/ARM/MCTargetDesc/ARMMCTargetDesc.h
>   llvm/trunk/lib/Target/BPF/BPFSubtarget.cpp
>   llvm/trunk/lib/Target/BPF/MCTargetDesc/BPFAsmBackend.cpp
>   llvm/trunk/lib/Target/BPF/MCTargetDesc/BPFMCAsmInfo.h
>   llvm/trunk/lib/Target/BPF/MCTargetDesc/BPFMCTargetDesc.cpp
>   llvm/trunk/lib/Target/BPF/MCTargetDesc/BPFMCTargetDesc.h
>   llvm/trunk/lib/Target/Hexagon/HexagonSubtarget.cpp
>   llvm/trunk/lib/Target/Hexagon/MCTargetDesc/HexagonAsmBackend.cpp
>   llvm/trunk/lib/Target/Hexagon/MCTargetDesc/HexagonMCAsmInfo.cpp
>   llvm/trunk/lib/Target/Hexagon/MCTargetDesc/HexagonMCAsmInfo.h
>   llvm/trunk/lib/Target/Hexagon/MCTargetDesc/HexagonMCTargetDesc.cpp
>   llvm/trunk/lib/Target/Hexagon/MCTargetDesc/HexagonMCTargetDesc.h
>   llvm/trunk/lib/Target/MSP430/MCTargetDesc/MSP430MCAsmInfo.cpp
>   llvm/trunk/lib/Target/MSP430/MCTargetDesc/MSP430MCAsmInfo.h
>   llvm/trunk/lib/Target/MSP430/MCTargetDesc/MSP430MCTargetDesc.cpp
>   llvm/trunk/lib/Target/MSP430/MSP430Subtarget.cpp
>   llvm/trunk/lib/Target/Mips/AsmParser/MipsAsmParser.cpp
>   llvm/trunk/lib/Target/Mips/MCTargetDesc/MipsABIInfo.cpp
>   llvm/trunk/lib/Target/Mips/MCTargetDesc/MipsABIInfo.h
>   llvm/trunk/lib/Target/Mips/MCTargetDesc/MipsAsmBackend.cpp
>   llvm/trunk/lib/Target/Mips/MCTargetDesc/MipsAsmBackend.h
>   llvm/trunk/lib/Target/Mips/MCTargetDesc/MipsMCAsmInfo.cpp
>   llvm/trunk/lib/Target/Mips/MCTargetDesc/MipsMCAsmInfo.h
>   llvm/trunk/lib/Target/Mips/MCTargetDesc/MipsMCTargetDesc.cpp
>   llvm/trunk/lib/Target/Mips/MCTargetDesc/MipsMCTargetDesc.h
>   llvm/trunk/lib/Target/Mips/MipsAsmPrinter.cpp
>   llvm/trunk/lib/Target/Mips/MipsSubtarget.cpp
>   llvm/trunk/lib/Target/Mips/MipsTargetMachine.cpp
>   llvm/trunk/lib/Target/NVPTX/MCTargetDesc/NVPTXMCAsmInfo.cpp
>   llvm/trunk/lib/Target/NVPTX/MCTargetDesc/NVPTXMCAsmInfo.h
>   llvm/trunk/lib/Target/NVPTX/MCTargetDesc/NVPTXMCTargetDesc.cpp
>   llvm/trunk/lib/Target/NVPTX/NVPTXSubtarget.cpp
>   llvm/trunk/lib/Target/PowerPC/AsmParser/PPCAsmParser.cpp
>   llvm/trunk/lib/Target/PowerPC/MCTargetDesc/PPCAsmBackend.cpp
>   llvm/trunk/lib/Target/PowerPC/MCTargetDesc/PPCMCAsmInfo.cpp
>   llvm/trunk/lib/Target/PowerPC/MCTargetDesc/PPCMCAsmInfo.h
>   llvm/trunk/lib/Target/PowerPC/MCTargetDesc/PPCMCCodeEmitter.cpp
>   llvm/trunk/lib/Target/PowerPC/MCTargetDesc/PPCMCTargetDesc.cpp
>   llvm/trunk/lib/Target/PowerPC/MCTargetDesc/PPCMCTargetDesc.h
>   llvm/trunk/lib/Target/PowerPC/PPCSubtarget.cpp
>   llvm/trunk/lib/Target/Sparc/AsmParser/SparcAsmParser.cpp
>   llvm/trunk/lib/Target/Sparc/MCTargetDesc/SparcAsmBackend.cpp
>   llvm/trunk/lib/Target/Sparc/MCTargetDesc/SparcMCAsmInfo.cpp
>   llvm/trunk/lib/Target/Sparc/MCTargetDesc/SparcMCAsmInfo.h
>   llvm/trunk/lib/Target/Sparc/MCTargetDesc/SparcMCTargetDesc.cpp
>   llvm/trunk/lib/Target/Sparc/MCTargetDesc/SparcMCTargetDesc.h
>   llvm/trunk/lib/Target/Sparc/SparcSubtarget.cpp
>
> llvm/trunk/lib/Target/SystemZ/MCTargetDesc/SystemZMCAsmBackend.cpp
>   llvm/trunk/lib/Target/SystemZ/MCTargetDesc/SystemZMCAsmInfo.cpp
>   llvm/trunk/lib/Target/SystemZ/MCTargetDesc/SystemZMCAsmInfo.h
>   llvm/trunk/lib/Target/SystemZ/MCTargetDesc/SystemZMCTargetDesc.cpp
>   llvm/trunk/lib/Target/SystemZ/MCTargetDesc/SystemZMCTargetDesc.h
>   llvm/trunk/lib/Target/SystemZ/SystemZSubtarget.cpp
>   llvm/trunk/lib/Target/TargetSubtargetInfo.cpp
>   llvm/trunk/lib/Target/X86/AsmParser/X86AsmInstrumentation.cpp
>   llvm/trunk/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
>   llvm/trunk/lib/Target/X86/MCTargetDesc/X86MCAsmInfo.cpp
>   llvm/trunk/lib/Target/X86/MCTargetDesc/X86MCAsmInfo.h
>   llvm/trunk/lib/Target/X86/MCTargetDesc/X86MCTargetDesc.cpp
>   llvm/trunk/lib/Target/X86/MCTargetDesc/X86MCTargetDesc.h
>   llvm/trunk/lib/Target/X86/X86RegisterInfo.cpp
>   llvm/trunk/lib/Target/X86/X86Subtarget.cpp
>   llvm/trunk/lib/Target/XCore/MCTargetDesc/XCoreMCAsmInfo.cpp
>   llvm/trunk/lib/Target/XCore/MCTargetDesc/XCoreMCAsmInfo.h
>   llvm/trunk/lib/Target/XCore/MCTargetDesc/XCoreMCTargetDesc.cpp
>   llvm/trunk/lib/Target/XCore/XCoreSubtarget.cpp
>   llvm/trunk/tools/dsymutil/DwarfLinker.cpp
>   llvm/trunk/tools/llvm-mc/llvm-mc.cpp
>   llvm/trunk/tools/llvm-objdump/MachODump.cpp
>   llvm/trunk/tools/llvm-objdump/llvm-objdump.cpp
>   llvm/trunk/tools/llvm-rtdyld/llvm-rtdyld.cpp
>   llvm/trunk/utils/TableGen/SubtargetEmitter.cpp

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150915/696eb89d/attachment.html>


More information about the llvm-commits mailing list