[PATCH] D17183: Make TargetInfo store an actual DataLayout instead of a string.

dexonsmith added a comment. Maybe LLVM should grow an `LLVMTargetInfo` library that sits between `LLVMSupport` and `LLVMCore`, as a version of solution 2 that doesn't induce more bloating in Support. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D17183/new/

[PATCH] D17183: Make TargetInfo store an actual DataLayout instead of a string.

echristo added a comment. In D17183#2656771 , @jyknight wrote: > In general, I think it's extremely unfortunate that Clang and LLVM have > different copies of the same information. It's a problem for way more than > just this one situation. So, I really

[PATCH] D17183: Make TargetInfo store an actual DataLayout instead of a string.

jyknight added a comment. In general, I think it's extremely unfortunate that Clang and LLVM have different copies of the same information. It's a problem for way more than just this one situation. So, I really don't like choice 1 -- I think it's moving in the wrong direction. The recent

[PATCH] D17183: Make TargetInfo store an actual DataLayout instead of a string.

rnk added a comment. I think 1 is OK as long as we can assert that things don't get out of sync. I like the idea of putting the prefix next to the data layout string. In the long run, splitting up Support might be reasonable, and pushing DataLayout down out of IR seems like something that

[PATCH] D17183: Make TargetInfo store an actual DataLayout instead of a string.

thakis added a comment. Zombie comment time! I kind of don't love this change. clang's libBasic shouldn't depend on llvm/lib/IR: Practically there's no reason that building clang-format should build lib/IR and its deps, and more importantly semantically the dependency feels off too. The dep

Re: [PATCH] D17183: Make TargetInfo store an actual DataLayout instead of a string.

This revision was automatically updated to reflect the committed changes. jyknight marked an inline comment as done. Closed by commit rL262737: Make TargetInfo store an actual DataLayout instead of a string. (authored by jyknight). Changed prior to commit:

Re: [PATCH] D17183: Make TargetInfo store an actual DataLayout instead of a string.

rafael added a subscriber: rafael. rafael added a comment. This is awesome! Comment at: include/clang/CodeGen/BackendUtil.h:38 @@ -37,3 +37,3 @@ const TargetOptions , const LangOptions , - StringRef TDesc, llvm::Module *M,

Re: [PATCH] D17183: Make TargetInfo store an actual DataLayout instead of a string.

echristo accepted this revision. echristo added a comment. This revision is now accepted and ready to land. Sorry, I've been waffling on this, but I think this is fine. Thanks! -eric http://reviews.llvm.org/D17183 ___ cfe-commits mailing list

Re: [PATCH] D17183: Make TargetInfo store an actual DataLayout instead of a string.

jyknight added a comment. Ping. I'm pretty sure you said you were going to look at this. :) http://reviews.llvm.org/D17183 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D17183: Make TargetInfo store an actual DataLayout instead of a string.

probinson added a subscriber: probinson. probinson added a comment. Driveby comment: the changes in unittests look unrelated? http://reviews.llvm.org/D17183 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

Re: [PATCH] D17183: Make TargetInfo store an actual DataLayout instead of a string.

Those changes were necessary due to a name (IIRC it was "Module") existing in both the clang and llvm namespaces, and after this change, becoming ambiguous in those files. On Feb 12, 2016 3:16 AM, "Paul Robinson" wrote: > probinson added a subscriber:

Re: [PATCH] D17183: Make TargetInfo store an actual DataLayout instead of a string.

probinson added a comment. In http://reviews.llvm.org/D17183#351361, @jyknight wrote: > Those changes were necessary due to a name (IIRC it was "Module") existing > in both the clang and llvm namespaces, and after this change, becoming > ambiguous in those files. Ah, that one. Okay.

[PATCH] D17183: Make TargetInfo store an actual DataLayout instead of a string.

jyknight created this revision. jyknight added a reviewer: echristo. jyknight added a subscriber: cfe-commits. Use it to calculate UserLabelPrefix, instead of specifying it (often incorrectly). Note that the *actual* user label prefix has always come from the DataLayout, and is handled within

Re: [PATCH] D17183: Make TargetInfo store an actual DataLayout instead of a string.

yaron.keren added a subscriber: yaron.keren. yaron.keren added a comment. We have tried to keep one copy of http://reviews.llvm.org/D11103 Can it share the Module->getDataLayout() ? http://reviews.llvm.org/D17183 ___ cfe-commits mailing list

Re: [PATCH] D17183: Make TargetInfo store an actual DataLayout instead of a string.

jyknight added a comment. In http://reviews.llvm.org/D17183#351065, @yaron.keren wrote: > We have tried to keep one copy of DataLayout around > http://reviews.llvm.org/D11103 > Can it share the Module->getDataLayout() ? We're at the beginning of clang here, before preprocessing. There's no