[PATCH] D14687: Macro support in LLVM IR

[Duncan P. N. Exon Smith via llvm-commits]

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
> 
> 
>