[PATCH] D14687: Macro support in LLVM IR
Duncan P. N. Exon Smith via llvm-commits
llvm-commits at lists.llvm.org
Wed Dec 2 10:05:58 PST 2015
Sorry for the delay, was away all last week and haven't made it back
through my inbox yet. I just saw your ping to Alexey.
> On 2015-Nov-23, at 05:55, Amjad Aboud <amjad.aboud at intel.com> wrote:
>
> aaboud added a comment.
>
> Thanks for the comments.
> Please, see my answers below.
>
>
> ================
> Comment at: docs/LangRef.rst:3696
> @@ -3696,2 +3695,3 @@
> +unit, regardless of code optimizations (some nodes are only emitted if there are
> references to them from instructions).
>
> ----------------
>> I assume these line endings are an artifact of the diff, and won't be committed?
> If you mean these lines, then they will be committed, actually, this was in the rst file before my changes.
> But, I think it will not be shown in the HTML LangRef document.
>
> ================
> Comment at: lib/Bitcode/Reader/BitcodeReader.cpp:2207
> @@ -2207,1 +2206,3 @@
> + getMDOrNull(Record[13]), Record.size() <= 14 ? 0 : Record[14],
> + Record.size() <= 15 ? 0 : getMDOrNull(Record[15])),
> NextMDValueNo++);
> ----------------
>> The canonical place for `macros:` should really be *before* `dwoId`.
>> It's one of the set of arrays. Same probably makes sense for the C++ APIs. The most important thing is LangRef and AsmWriter. Please move it!
> I tried that approach at first. However, it will not work, as it will break the backward compatibility.
> There are some LIT tests that have 'dwoId' as record number 14, if I change the order these bitcode files will fail to load using the new bitcode reader.
>
> Do you think that I should not care about backward compatibility and change the order back such that 'macros' appear before 'dwoId'?
Any requirements for bitcode record order are unrelated to textual IR
syntax and C++ API. Do what you have to do to read old bitcode, but
that shouldn't block making the C++ API and textual IR natural.
I don't really care where the macros show up in the bitcode records,
since there are relatively few humans that read those.
>
>
>
> Repository:
> rL LLVM
>
> http://reviews.llvm.org/D14687
>
>
>
More information about the llvm-commits
mailing list