[PATCH] Debug Info Verifier pass

[Eric Christopher]

On Sat, Apr 12, 2014 at 3:13 PM, Duncan P. N. Exon Smith
<dexonsmith at apple.com> wrote:
> Thanks for the reviews, Eric and Manman.  I'll send new patches probably
> on Monday (after I get a chance to see what happens with make check-all),
> but in the meantime:
>
> On 2014 Apr 11, at 15:17, Eric Christopher <echristo at gmail.com> wrote:
>
>> 1) The Broken variable seems weird.
>>
>> An idea for a refactor:
>>
>> +  // CheckFailed - A check failed, so print out the condition and the message
>> +  // that failed.  This provides a nice place to put a breakpoint if you want
>> +  // to see why something is not correct.
>> +  void CheckFailed(const Twine &Message, const Value *V1 = 0,
>> +                   const Value *V2 = 0, const Value *V3 = 0,
>> +                   const Value *V4 = 0) {
>> +    OS << Message.str() << "\n";
>> +    WriteValue(V1);
>> +    WriteValue(V2);
>> +    WriteValue(V3);
>> +    WriteValue(V4);
>> +    Broken = true;
>> +  }
>> +
>>
>> Maybe just use a variadic template, iterate over the arguments, and
>> have "WriteValue" just take either a type or a value? I think that
>> might work?
>
> Frankly, I'd like to avoid a refactor... I just moved this code
> (verbatim) out of Verifier so I could use the same macros in
> DebugInfoVerifier.  Can we punt that until later?
>

Sure :)

>> Can we use those yet?
>
> Looks like not :(.
>
> http://llvm.org/docs/CodingStandards.html#supported-c-11-language-and-library-features
>

Sad, no worries though.

>> 2)
>>
>> void LLVMAddVerifierPass(LLVMPassManagerRef PM) {
>>   unwrap(PM)->add(createVerifierPass());
>> +  // FIXME: should this also add createDebugInfoVerifierPass()?
>> }
>>
>> ... I'm not even sure why this is here? (Not your code, that comment
>> is fine, the original though...)
>
> This seems to be for the OCaml bindings:
>
> $ tail -n +167 bindings/ocaml/transforms/scalar/scalar_opts_ocaml.c | head -n 5
> /* [<Llvm.PassManager.any] Llvm.PassManager.t -> unit */
> CAMLprim value llvm_add_verifier(LLVMPassManagerRef PM) {
>   LLVMAddVerifierPass(PM);
>   return Val_unit;
> }
>
> Is the FIXME is sufficient, or should I dig deeper?

Nope. That's fine. I was just curious.

-eric