Skip to content

Commit

Permalink
[Codegen_LLVM] Arguments/Loads/Returns are well-defined
Browse files Browse the repository at this point in the history
https://llvm.org/docs/LangRef.html#well-defined-values:
```
Given a program execution, a value is well defined if the value
does not have an undef bit and is not poison in the execution.
An aggregate value or vector is well defined if its elements
are well defined. The padding of an aggregate isn’t considered,
since it isn’t visible without storing it into memory and
loading it with a different type.
```

Same story as with halide#6897,
this matches //my// understanding of Halide's semantics,
but this may very well not be what the reality is.

To be noted, even //if// this is correct, i do believe
this has a very high chance to cause issues.
I'm also not sure how this plays with MSan.

Notably, while annotating function arguments/return is trivial,
for load's this only annotates the `load` instructions that we
actually create, i.e. it misses loads in the IR that we precompiled
and then link into our module.
  • Loading branch information
LebedevRI committed Aug 1, 2022
1 parent 703a738 commit 84dfa51
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 8 deletions.
10 changes: 10 additions & 0 deletions src/CodeGen_Internal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -711,6 +711,16 @@ void set_function_attributes_from_halide_target_options(llvm::Function &fn) {
// But asserts and external calls *might* abort.
fn.setMustProgress();

// All parameters and return values are well-defined,
// do not have any undef bits, and are not poison in execution.
if (!fn.getReturnType()->isVoidTy())
fn.addRetAttr(Attribute::NoUndef);
for (Argument &A : fn.args()) {
if (!A.hasAttribute(Attribute::ImmArg)) {
A.addAttr(Attribute::NoUndef);
}
}

// Turn off approximate reciprocals for division. It's too
// inaccurate even for us.
fn.addFnAttr("reciprocal-estimates", "none");
Expand Down
2 changes: 1 addition & 1 deletion src/CodeGen_Internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ namespace llvm {
class ConstantFolder;
class ElementCount;
class Function;
class IRBuilderDefaultInserter;
class IRBuilderCallbackInserter;
class LLVMContext;
class Module;
class StructType;
Expand Down
21 changes: 16 additions & 5 deletions src/CodeGen_LLVM.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -292,10 +292,21 @@ void CodeGen_LLVM::initialize_llvm() {
});
}

void CodeGen_LLVM::irbuilder_inserter_callback(llvm::Instruction *I) {
if (auto *load = dyn_cast<LoadInst>(I)) {
// All loads are well-defined, do not have any undef bits,
// and are not poison in execution.
I->setMetadata(LLVMContext::MD_noundef,
llvm::MDNode::get(I->getContext(), {}));
}
}

void CodeGen_LLVM::init_context() {
// Ensure our IRBuilder is using the current context.
delete builder;
builder = new IRBuilder<>(*context);
builder =
new IRBuilderTy(*context, ConstantFolder(),
IRBuilderCallbackInserter(irbuilder_inserter_callback));

// Branch weights for very likely branches
llvm::MDBuilder md_builder(*context);
Expand Down Expand Up @@ -3184,7 +3195,7 @@ void CodeGen_LLVM::visit(const Call *op) {
llvm::DataLayout d(module.get());
value = ConstantInt::get(i32_t, (int)d.getTypeAllocSize(halide_buffer_t_type));
} else if (op->is_intrinsic(Call::strict_float)) {
IRBuilder<llvm::ConstantFolder, llvm::IRBuilderDefaultInserter>::FastMathFlagGuard guard(*builder);
IRBuilderTy::FastMathFlagGuard guard(*builder);
llvm::FastMathFlags safe_flags;
safe_flags.clear();
builder->setFastMathFlags(safe_flags);
Expand Down Expand Up @@ -3240,7 +3251,7 @@ void CodeGen_LLVM::visit(const Call *op) {
* treated as strict. Note that compilation may still be in
* fast-math mode due to global options, but that's ok due to
* the aforementioned special casing. */
IRBuilder<llvm::ConstantFolder, llvm::IRBuilderDefaultInserter>::FastMathFlagGuard guard(*builder);
IRBuilderTy::FastMathFlagGuard guard(*builder);
llvm::FastMathFlags safe_flags;
safe_flags.clear();
builder->setFastMathFlags(safe_flags);
Expand All @@ -3251,7 +3262,7 @@ void CodeGen_LLVM::visit(const Call *op) {
(op->name == "is_inf_f32" || op->name == "is_inf_f64" || op->name == "is_inf_f16")) {
internal_assert(op->args.size() == 1);

IRBuilder<llvm::ConstantFolder, llvm::IRBuilderDefaultInserter>::FastMathFlagGuard guard(*builder);
IRBuilderTy::FastMathFlagGuard guard(*builder);
llvm::FastMathFlags safe_flags;
safe_flags.clear();
builder->setFastMathFlags(safe_flags);
Expand All @@ -3267,7 +3278,7 @@ void CodeGen_LLVM::visit(const Call *op) {
internal_assert(op->args.size() == 1);
internal_assert(op->args[0].type().is_float());

IRBuilder<llvm::ConstantFolder, llvm::IRBuilderDefaultInserter>::FastMathFlagGuard guard(*builder);
IRBuilderTy::FastMathFlagGuard guard(*builder);
llvm::FastMathFlags safe_flags;
safe_flags.clear();
builder->setFastMathFlags(safe_flags);
Expand Down
7 changes: 5 additions & 2 deletions src/CodeGen_LLVM.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ class Value;
class Module;
class Function;
class FunctionType;
class IRBuilderDefaultInserter;
class IRBuilderCallbackInserter;
class ConstantFolder;
template<typename, typename>
class IRBuilder;
Expand Down Expand Up @@ -159,7 +159,10 @@ class CodeGen_LLVM : public IRVisitor {
std::unique_ptr<llvm::Module> module;
llvm::Function *function;
llvm::LLVMContext *context;
llvm::IRBuilder<llvm::ConstantFolder, llvm::IRBuilderDefaultInserter> *builder;
using IRBuilderTy =
llvm::IRBuilder<llvm::ConstantFolder, llvm::IRBuilderCallbackInserter>;
static void irbuilder_inserter_callback(llvm::Instruction *I);
IRBuilderTy *builder;
llvm::Value *value;
llvm::MDNode *very_likely_branch;
llvm::MDNode *default_fp_math_md;
Expand Down

0 comments on commit 84dfa51

Please sign in to comment.