[PATCH] D25265: [C API] Add test for D25259 and new LLVMIsExact function.

[Manuel Jacob via llvm-commits]

mjacob added a comment.

In https://reviews.llvm.org/D25265#562294, @mehdi_amini wrote:

> In https://reviews.llvm.org/D25265#562258, @mjacob wrote:
>
> > - It adds a new C API function (`LLVMIsExact`) which I don't really care about other than for writing this test.
> > - It's asymmetric (e.g. missing something like `LLVMSetIsExact`). @deadalnix pointed this out as well.
>
>
> The general question I'd have would be: isn't the ability to round-trip something that is "useful" in general with the C API?
>  If yes then it seems that the test helped identified the missing `LLVMIsExact`, and it is legit.
>  Otherwise yeah it isn't great to add API just for being able to fit the testing infrastructure and fit other APIs.


Yes, being able to roundtrip is useful. The obvious roundtrip would be through `LLVMIsExact` and `LLVMSetIsExact`. `LLVMSetIsExact` and `LLVMBuildExactUDiv` is less obvious and is what I meant with "asymmetric".

Sure, if the point was to add `LLVMIsExact` and `LLVMSetIsExact`, the echo test would probably be the perfect place to test this.

>> - LLVMConstExactUDiv is still untested.
> 
> Can't you round-trip the following?
> 
>   @foo = external global i8
>   define i64 @test() {
>     ret i64 udiv exact (i64 ptrtoint (i8* @foo to i64), i64 10)
>   }

Yes, something like that could be added if we decide that the echo test is the right place to add it to.

>> I think we need a different approach for testing the C API. Something like calling a bunch of C API functions, dumping the module and checking it with FileCheck.
> 
> I'd be fine with that, but do we have the infrastructure for it?

I tried this and I think it should work with the current infrastructure. We'd have a file `tools/llvm-c-test/test_ir.c` with a lot of code like this:

  // CHECK-NEXT: %add = add i64 %x, %y
  LLVMBuildAdd(Builder, X, Y, "add");

and a file `test/Bindings/llvm-c/test_ir.test` with only the following content:

  RUN: llvm-c-test --test-ir 2>&1 | FileCheck %S/../../../tools/llvm-c-test/test_ir.c

(or however this can be expressed cleaner).


https://reviews.llvm.org/D25265