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

[deadalnix]

Summary

The LLVMIsExact function was added to make this test possible, but it is
generally useful.

[deadalnix]

include/llvm-c/Core.h

3289–3290

Don't put that in the middle of the builder mapping.

tools/llvm-c-test/echo.cpp

503–506

This begs the question: should this have been done via a SetExact function ?

[mjacob]

I wasn't very happy with the approach already when submitting the patch.

• 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.
• LLVMConstExactUDiv is still untested.

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.

[mehdi_amini]

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

• 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)
}

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?

[mjacob]

In D25265#562294, @mehdi_amini wrote:

In 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).

[mehdi_amini]

In D25265#562311, @mjacob wrote:

In D25265#562294, @mehdi_amini wrote:

In 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".

I understood the "asymmetric issue", unfortunately that's something that comes up at the C++ API level as well, so it is not really a C API issue here.

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

My point was rather: the fact that the test is about round-trip pointed a "hole" in the API with the lack of "setIsExact".

[deadalnix]

Pinging this. I think this is fine and should proceed.

[deadalnix]

Ping ?

[deadalnix]

Ping ?

[mjacob]

I'm really sorry to not have finished this, despite telling you that I'll do it.

The one remaining issue: where do we move the LLVMIsExact to? After LLVMBuildExactSDiv? Something similar was done with LLVMIsAtomicSingleThread, which is right after the LLVMBuildAtomicCmpXchg that creates atomic instructions.

[whitequark]

I think we need to add LLVMSetExact as well.

include/llvm-c/Core.h

 LLVMValueRef LLVMBuildNeg(LLVMBuilderRef, LLVMValueRef V, const char *Name);
 LLVMValueRef LLVMBuildNSWNeg(LLVMBuilderRef B, LLVMValueRef V,
                              const char *Name);
 LLVMValueRef LLVMBuildNUWNeg(LLVMBuilderRef B, LLVMValueRef V,
                              const char *Name);
 LLVMValueRef LLVMBuildFNeg(LLVMBuilderRef, LLVMValueRef V, const char *Name);
 LLVMValueRef LLVMBuildNot(LLVMBuilderRef, LLVMValueRef V, const char *Name);
 
+LLVMBool LLVMIsExact(LLVMValueRef V);
+
 /* Memory */
 LLVMValueRef LLVMBuildMalloc(LLVMBuilderRef, LLVMTypeRef Ty, const char *Name);
 LLVMValueRef LLVMBuildArrayMalloc(LLVMBuilderRef, LLVMTypeRef Ty,
                                   LLVMValueRef Val, const char *Name);
 LLVMValueRef LLVMBuildAlloca(LLVMBuilderRef, LLVMTypeRef Ty, const char *Name);
 LLVMValueRef LLVMBuildArrayAlloca(LLVMBuilderRef, LLVMTypeRef Ty,
                                   LLVMValueRef Val, const char *Name);
 LLVMValueRef LLVMBuildFree(LLVMBuilderRef, LLVMValueRef PointerVal);

lib/IR/Core.cpp

 LLVMValueRef LLVMBuildFNeg(LLVMBuilderRef B, LLVMValueRef V, const char *Name) {
   return wrap(unwrap(B)->CreateFNeg(unwrap(V), Name));
 }
 
 LLVMValueRef LLVMBuildNot(LLVMBuilderRef B, LLVMValueRef V, const char *Name) {
   return wrap(unwrap(B)->CreateNot(unwrap(V), Name));
 }
 
+LLVMBool LLVMIsExact(LLVMValueRef V) {
+    return cast<PossiblyExactOperator>(unwrap(V))->isExact();
+}
+
 /*--.. Memory ..............................................................--*/
 
 LLVMValueRef LLVMBuildMalloc(LLVMBuilderRef B, LLVMTypeRef Ty,
                              const char *Name) {
   Type* ITy = Type::getInt32Ty(unwrap(B)->GetInsertBlock()->getContext());
   Constant* AllocSize = ConstantExpr::getSizeOf(unwrap(Ty));
   AllocSize = ConstantExpr::getTruncOrBitCast(AllocSize, ITy);
   Instruction* Malloc = CallInst::CreateMalloc(unwrap(B)->GetInsertBlock(),

test/Bindings/llvm-c/echo.ll

 }
 
 define i32 @iops(i32 %a, i32 %b) {
   %1 = add i32 %a, %b
   %2 = mul i32 %a, %1
   %3 = sub i32 %2, %1
   %4 = udiv i32 %3, %b
   %5 = sdiv i32 %2, %4
+  %e4 = udiv exact i32 %3, %b
+  %e5 = sdiv exact i32 %2, %4
   %6 = urem i32 %3, %5
   %7 = srem i32 %2, %6
   %8 = shl i32 %1, %b
   %9 = lshr i32 %a, %7
   %10 = ashr i32 %b, %8
   %11 = and i32 %9, %10
   %12 = or i32 %2, %11
   %13 = xor i32 %12, %4

tools/llvm-c-test/echo.cpp

switch(Op) {
         LLVMValueRef LHS = CloneValue(LLVMGetOperand(Src, 0));
         LLVMValueRef RHS = CloneValue(LLVMGetOperand(Src, 1));
         Dst = LLVMBuildMul(Builder, LHS, RHS, Name);
         break;
       }
       case LLVMUDiv: {
         LLVMValueRef LHS = CloneValue(LLVMGetOperand(Src, 0));
         LLVMValueRef RHS = CloneValue(LLVMGetOperand(Src, 1));
+        if (LLVMIsExact(Src))
+            Dst = LLVMBuildExactUDiv(Builder, LHS, RHS, Name);
+        else
             Dst = LLVMBuildUDiv(Builder, LHS, RHS, Name);
         break;
       }
       case LLVMSDiv: {
         LLVMValueRef LHS = CloneValue(LLVMGetOperand(Src, 0));
         LLVMValueRef RHS = CloneValue(LLVMGetOperand(Src, 1));
+        if (LLVMIsExact(Src))
+            Dst = LLVMBuildExactSDiv(Builder, LHS, RHS, Name);
+        else
             Dst = LLVMBuildSDiv(Builder, LHS, RHS, Name);
         break;
       }
       case LLVMURem: {
         LLVMValueRef LHS = CloneValue(LLVMGetOperand(Src, 0));
         LLVMValueRef RHS = CloneValue(LLVMGetOperand(Src, 1));
         Dst = LLVMBuildURem(Builder, LHS, RHS, Name);
         break;
       }