[builtins] Add __builtin_assume_separate_storage.

[davidtgoldblatt]

Summary

Just plumbing from the language level to the assume intrinsics with
separate_storage operand bundles.

[davidtgoldblatt]

Updating per review comments.

[davidtgoldblatt]

Updating per review comments.

[davidtgoldblatt]

Fix out-of-date comment.

[davidtgoldblatt]

Per review comments.

[davidtgoldblatt]

Rebase

[bruno]

Nice, thanks for adding the builtin layer.

clang/lib/Sema/SemaChecking.cpp

7808 ↗

(On Diff #481770)

Can you prefix this with a FIXME/TODO?

7810 ↗

(On Diff #481770)

IIUC, seems like there a two pieces here you can already cover as part of this patch:

• Check for exact two arguments. If in the future an optional multi-arg variadic version is to be supported the checking could be enhanced/changed to support that.
• Check both are pointers. I noticed that in the testcase you rely on incompatible integer to pointer conversion and friends to catch some of these, but a more user friendly diagnostic could be explicit about the expectations. In case these current error diagnostics you are relying run before this sema check, perhaps a note diagnostic could help clarify.

[bruno]

Additionally, this likely deserves an entry in ./clang/docs/ReleaseNotes.rst.

[davidtgoldblatt]

Summarizing a brief offline discussion: I think it's probably fine to just get rid of the SemaChecking changes entirely; type-checking here is done by the generalized builtin-handling code. I'll do the change if there's no objection.

[davidtgoldblatt]

Per comments.

[davidtgoldblatt]

In addition to the release notes addition, I also added an entry in LanguageExtensions.rst.

[davidtgoldblatt]

Use utils/update_cc_test_checks.py for the codegen test.

[bruno]

clang/docs/LanguageExtensions.rst

2395

Not necessarily a blocker, but it seems like some of these things you mention can actually be caught by diagnostics without too much effort? Any plans to add them?

[davidtgoldblatt]

clang/docs/LanguageExtensions.rst

2395

I hadn't planned to (at least in the short term). Practically I expect uses of this to be mostly backed out from looking at bad assembly (this is the way I've been using it so far in experimentation). You wouldn't generally expect people to want to try to express "these two struct fields don't alias" and so on because alias analysis can already handle those cases fairly well.

My inclination would be to wait on the diagnostics until we see if it's a real problem, but I'm not strongly opposed if you'd really like them in v1. (Although in that case I'll probably bug you for some help with where / how to put the diagnostics).

[bruno]

LGTM!

clang/docs/LanguageExtensions.rst

2395

My inclination would be to wait on the diagnostics until we see if it's a real problem

Sounds fair! Feel free to bug me when/if you decide to explore that route.

[davidtgoldblatt]

Rebase

[OfekShilon]

clang/docs/LanguageExtensions.rst

2395

+2c: as of today tbaa *doesn't* handle well aliasing of two struct fields. There was some work to mitigate it by @kosarev (https://reviews.llvm.org/D41539) but it is still hidden behind a -new-struct-path-tbaa switch. Example: https://godbolt.org/z/xPMzfea8W

clang/docs/LanguageExtensions.rst

 The boolean argument to this function is defined to be true. The optimizer may
 analyze the form of the expression provided as the argument and deduce from
 that information used to optimize the program. If the condition is violated
 during execution, the behavior is undefined. The argument itself is never
 evaluated, so any side effects of the expression will be discarded.
 
 Query for this feature with ``__has_builtin(__builtin_assume)``.
 
+.. _langext-__builtin_assume_separate_storage:
+
+``__builtin_assume_separate_storage``
+--------------------
+
+``__builtin_assume_separate_storage`` is used to provide the optimizer with the
+knowledge that its two arguments point to separately allocated objects.
+
+**Syntax**:
+
+.. code-block:: c++
+
+    __builtin_assume_separate_storage(const volatile void *, const volatile void *)
+
+**Example of Use**:
+
+.. code-block:: c++
+
+  int foo(int *x, int *y) {
+      __builtin_assume_separate_storage(x, y);
+      *x = 0;
+      *y = 1;
+      // The optimizer may optimize this to return 0 without reloading from *x.
+      return *x;
+  }
+
+**Description**:
+
+The arguments to this function are assumed to point into separately allocated
+storage (either different variable definitions or different dynamic storage
+allocations). The optimizer may use this fact to aid in alias analysis. If the
+arguments point into the same storage, the behavior is undefined. Note that the
+definition of "storage" here refers to the outermost enclosing allocation of any
+particular object (so for example, it's never correct to call this function
+passing the addresses of fields in the same struct, elements of the same array,
+etc.).
+
+Query for this feature with ``__has_builtin(__builtin_assume_separate_storage)``.
+
+
 ``__builtin_offsetof``
 ----------------------
 
 ``__builtin_offsetof`` is used to implement the ``offsetof`` macro, which
 calculates the offset (in bytes) to a given member of the given type.
 
 **Syntax**:
 

clang/docs/ReleaseNotes.rst

 - Clang now saves the address of ABI-indirect function parameters on the stack,
   improving the debug information available in programs compiled without
   optimizations.
 - Clang now supports ``__builtin_nondeterministic_value`` that returns a
   nondeterministic value of the same type as the provided argument.
 - Clang now supports ``__builtin_FILE_NAME()`` which returns the same
   information as the ``__FILE_NAME__`` macro (the presumed file name
   from the invocation point, with no path components included).
+- Clang now supports ``__builtin_assume_separate_storage`` that indicates that
+  its arguments point to objects in separate storage allocations.
 
 New Compiler Flags
 ------------------
 
 Deprecated Compiler Flags
 -------------------------
 
 Modified Compiler Flags

clang/include/clang/Basic/Builtins.def

 LIBBUILTIN(move, "v&v&", "zfncThE", UTILITY, CXX_LANG)
 LIBBUILTIN(move_if_noexcept, "v&v&", "zfncThE", UTILITY, CXX_LANG)
 
 // Annotation function
 BUILTIN(__builtin_annotation, "v.", "tn")
 
 // Invariants
 BUILTIN(__builtin_assume, "vb", "nE")
+BUILTIN(__builtin_assume_separate_storage, "vvCD*vCD*", "nE")
 
 // Multiprecision Arithmetic Builtins.
 BUILTIN(__builtin_addcb, "UcUcCUcCUcCUc*", "n")
 BUILTIN(__builtin_addcs, "UsUsCUsCUsCUs*", "n")
 BUILTIN(__builtin_addc, "UiUiCUiCUiCUi*", "n")
 BUILTIN(__builtin_addcl, "ULiULiCULiCULiCULi*", "n")
 BUILTIN(__builtin_addcll, "ULLiULLiCULLiCULLiCULLi*", "n")
 BUILTIN(__builtin_subcb, "UcUcCUcCUcCUc*", "n")

clang/lib/CodeGen/CGBuiltin.cpp

case Builtin::BI__builtin_assume: {
     if (E->getArg(0)->HasSideEffects(getContext()))
       return RValue::get(nullptr);
 
     Value *ArgValue = EmitScalarExpr(E->getArg(0));
     Function *FnAssume = CGM.getIntrinsic(Intrinsic::assume);
     Builder.CreateCall(FnAssume, ArgValue);
     return RValue::get(nullptr);
   }
+  case Builtin::BI__builtin_assume_separate_storage: {
+    const Expr *Arg0 = E->getArg(0);
+    const Expr *Arg1 = E->getArg(1);
+
+    Value *Value0 = EmitScalarExpr(Arg0);
+    Value *Value1 = EmitScalarExpr(Arg1);
+
+    Value *Values[] = {Value0, Value1};
+    OperandBundleDefT<Value *> OBD("separate_storage", Values);
+    Builder.CreateAssumption(ConstantInt::getTrue(getLLVMContext()), {OBD});
+    return RValue::get(nullptr);
+  }
   case Builtin::BI__arithmetic_fence: {
     // Create the builtin call if FastMath is selected, and the target
     // supports the builtin, otherwise just return the argument.
     CodeGenFunction::CGFPOptionsRAII FPOptsRAII(*this, E);
     llvm::FastMathFlags FMF = Builder.getFastMathFlags();
     bool isArithmeticFenceEnabled =
         FMF.allowReassoc() &&
         getContext().getTargetInfo().checkArithmeticFenceSupported();

clang/test/CodeGen/builtin-assume-separate-storage.c

+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s | FileCheck %s
+void *nonconst(void);
+
+// CHECK-LABEL: @test1(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:    [[A_ADDR:%.*]] = alloca ptr, align 8
+// CHECK-NEXT:    [[B_ADDR:%.*]] = alloca ptr, align 8
+// CHECK-NEXT:    store ptr [[A:%.*]], ptr [[A_ADDR]], align 8
+// CHECK-NEXT:    store ptr [[B:%.*]], ptr [[B_ADDR]], align 8
+// CHECK-NEXT:    [[TMP0:%.*]] = load ptr, ptr [[A_ADDR]], align 8
+// CHECK-NEXT:    [[TMP1:%.*]] = load ptr, ptr [[B_ADDR]], align 8
+// CHECK-NEXT:    call void @llvm.assume(i1 true) [ "separate_storage"(ptr [[TMP0]], ptr [[TMP1]]) ]
+// CHECK-NEXT:    ret void
+//
+void test1(int *a, int *b) {
+
+  __builtin_assume_separate_storage(a, b);
+}
+
+// Separate storage assumptions evaluate their arguments unconditionally, like
+// assume_aligned but *unlike* assume. Check that we actually do so.
+// CHECK-LABEL: @test2(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:    [[A_ADDR:%.*]] = alloca ptr, align 8
+// CHECK-NEXT:    [[B_ADDR:%.*]] = alloca ptr, align 8
+// CHECK-NEXT:    store ptr [[A:%.*]], ptr [[A_ADDR]], align 8
+// CHECK-NEXT:    store ptr [[B:%.*]], ptr [[B_ADDR]], align 8
+// CHECK-NEXT:    [[TMP0:%.*]] = load ptr, ptr [[A_ADDR]], align 8
+// CHECK-NEXT:    [[CALL:%.*]] = call ptr @nonconst()
+// CHECK-NEXT:    call void @llvm.assume(i1 true) [ "separate_storage"(ptr [[TMP0]], ptr [[CALL]]) ]
+// CHECK-NEXT:    ret void
+//
+void test2(int *a, int *b) {
+  __builtin_assume_separate_storage(a, nonconst());
+}

clang/test/Sema/builtin-assume-separate-storage.c

+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsyntax-only -verify %s
+
+void *nonconst(void);
+
+void test1(int *a, int *b) {
+  __builtin_assume_separate_storage(a, b);
+  // Separate storage assumptions evaluate their arguments unconditionally, like
+  // assume_aligned but *unlike* assume. Check that we don't warn on it.
+  __builtin_assume_separate_storage(a, nonconst());
+  __builtin_assume_separate_storage(nonconst(), a);
+  __builtin_assume_separate_storage(a, 3); // expected-error {{incompatible integer to pointer conversion}}
+  __builtin_assume_separate_storage(3, a); // expected-error {{incompatible integer to pointer conversion}}
+}