[SimplifyCFG] Create attribute to disable simplifyCFG.

[morehouse]

Summary

When building with libFuzzer, simplifyCFG reduces the coverage signal
available to libFuzzer when trying to find new inputs. This patch
provides a way to disable simplifyCFG when building with libFuzzer.

Previous attempt https://reviews.llvm.org/D43423 received push-back
regarding the frontend flag.

[vitalybuka]

LGTM, but I'd wait a little for others inputs

llvm/lib/Transforms/IPO/ForceFunctionAttrs.cpp

46

no_simplify_cfg->nosimplifycfg ?

[hfinkel]

Do you actually want to disable all CFG simplifications, or do you just need to disable things that convert from control flow to something else (e.g., select formation)?

[arsenm]

I think the concept of an attribute to specially disable a specific pass is concerning. This is actually more aggressive since it will also disable it if a pass attempts to use the utility function on a specific block

[arsenm]

In D44057#1026419, @arsenm wrote:

I think the concept of an attribute to specially disable a specific pass is concerning. This is actually more aggressive since it will also disable it if a pass attempts to use the utility function on a specific block

More specifically I think if a pass wants to call simplifyCFG the utility function, it should be able to rely on some of the expected canonicalizations so it would not be OK for anything to just disable this

[morehouse]

In D44057#1026415, @hfinkel wrote:

Do you actually want to disable all CFG simplifications, or do you just need to disable things that convert from control flow to something else (e.g., select formation)?

Yes, what we need is to keep things as control flow so that our edge coverage picks up when a new condition is met. Is there a better way to do this?

In D44057#1026441, @arsenm wrote:

More specifically I think if a pass wants to call simplifyCFG the utility function, it should be able to rely on some of the expected canonicalizations so it would not be OK for anything to just disable this

An alternative would be to find every pass that calls simplifyCFG and disable them, but that may result in many optimizations unnecessarily disabled. Considering simplifyCFG can return false, shouldn't passes already be handling the case where simplification does not occur?

[morehouse]

In D44057#1026415, @hfinkel wrote:

Do you actually want to disable all CFG simplifications, or do you just need to disable things that convert from control flow to something else (e.g., select formation)?

Less aggressive alternative: https://reviews.llvm.org/D44232.

llvm/include/llvm/Bitcode/LLVMBitCodes.h

enum AttributeKindCodes {
   ATTR_KIND_NO_RECURSE = 48,
   ATTR_KIND_INACCESSIBLEMEM_ONLY = 49,
   ATTR_KIND_INACCESSIBLEMEM_OR_ARGMEMONLY = 50,
   ATTR_KIND_ALLOC_SIZE = 51,
   ATTR_KIND_WRITEONLY = 52,
   ATTR_KIND_SPECULATABLE = 53,
   ATTR_KIND_STRICT_FP = 54,
   ATTR_KIND_SANITIZE_HWADDRESS = 55,
+  ATTR_KIND_NO_SIMPLIFY_CFG = 56,
 };
 
 enum ComdatSelectionKindCodes {
   COMDAT_SELECTION_KIND_ANY = 1,
   COMDAT_SELECTION_KIND_EXACT_MATCH = 2,
   COMDAT_SELECTION_KIND_LARGEST = 3,
   COMDAT_SELECTION_KIND_NO_DUPLICATES = 4,
   COMDAT_SELECTION_KIND_SAME_SIZE = 5,

llvm/include/llvm/IR/Attributes.td

 def NoRecurse : EnumAttr<"norecurse">;
 
 /// Disable redzone.
 def NoRedZone : EnumAttr<"noredzone">;
 
 /// Mark the function as not returning.
 def NoReturn : EnumAttr<"noreturn">;
 
+/// CFG simplification is disabled.
+def NoSimplifyCFG : EnumAttr<"no_simplify_cfg">;
+
 /// Function doesn't unwind stack.
 def NoUnwind : EnumAttr<"nounwind">;
 
 /// opt_size.
 def OptimizeForSize : EnumAttr<"optsize">;
 
 /// Function must not be optimized.
 def OptimizeNone : EnumAttr<"optnone">;

llvm/lib/AsmParser/LLLexer.cpp

#define KEYWORD(STR)                                                           \
   KEYWORD(noduplicate);
   KEYWORD(noimplicitfloat);
   KEYWORD(noinline);
   KEYWORD(norecurse);
   KEYWORD(nonlazybind);
   KEYWORD(nonnull);
   KEYWORD(noredzone);
   KEYWORD(noreturn);
+  KEYWORD(no_simplify_cfg);
   KEYWORD(nounwind);
   KEYWORD(optnone);
   KEYWORD(optsize);
   KEYWORD(readnone);
   KEYWORD(readonly);
   KEYWORD(returned);
   KEYWORD(returns_twice);
   KEYWORD(signext);

llvm/lib/AsmParser/LLParser.cpp

while (true) {
     case lltok::kw_noduplicate: B.addAttribute(Attribute::NoDuplicate); break;
     case lltok::kw_noimplicitfloat:
       B.addAttribute(Attribute::NoImplicitFloat); break;
     case lltok::kw_noinline: B.addAttribute(Attribute::NoInline); break;
     case lltok::kw_nonlazybind: B.addAttribute(Attribute::NonLazyBind); break;
     case lltok::kw_noredzone: B.addAttribute(Attribute::NoRedZone); break;
     case lltok::kw_noreturn: B.addAttribute(Attribute::NoReturn); break;
     case lltok::kw_norecurse: B.addAttribute(Attribute::NoRecurse); break;
+    case lltok::kw_no_simplify_cfg:
+      B.addAttribute(Attribute::NoSimplifyCFG); break;
     case lltok::kw_nounwind: B.addAttribute(Attribute::NoUnwind); break;
     case lltok::kw_optnone: B.addAttribute(Attribute::OptimizeNone); break;
     case lltok::kw_optsize: B.addAttribute(Attribute::OptimizeForSize); break;
     case lltok::kw_readnone: B.addAttribute(Attribute::ReadNone); break;
     case lltok::kw_readonly: B.addAttribute(Attribute::ReadOnly); break;
     case lltok::kw_returns_twice:
       B.addAttribute(Attribute::ReturnsTwice); break;
     case lltok::kw_speculatable: B.addAttribute(Attribute::Speculatable); break;
while (true) {
     case lltok::kw_naked:
     case lltok::kw_nobuiltin:
     case lltok::kw_noduplicate:
     case lltok::kw_noimplicitfloat:
     case lltok::kw_noinline:
     case lltok::kw_nonlazybind:
     case lltok::kw_noredzone:
     case lltok::kw_noreturn:
+    case lltok::kw_no_simplify_cfg:
     case lltok::kw_nounwind:
     case lltok::kw_optnone:
     case lltok::kw_optsize:
     case lltok::kw_returns_twice:
     case lltok::kw_sanitize_address:
     case lltok::kw_sanitize_hwaddress:
     case lltok::kw_sanitize_memory:
     case lltok::kw_sanitize_thread:
while (true) {
     case lltok::kw_naked:
     case lltok::kw_nobuiltin:
     case lltok::kw_noduplicate:
     case lltok::kw_noimplicitfloat:
     case lltok::kw_noinline:
     case lltok::kw_nonlazybind:
     case lltok::kw_noredzone:
     case lltok::kw_noreturn:
+    case lltok::kw_no_simplify_cfg:
     case lltok::kw_nounwind:
     case lltok::kw_optnone:
     case lltok::kw_optsize:
     case lltok::kw_returns_twice:
     case lltok::kw_sanitize_address:
     case lltok::kw_sanitize_hwaddress:
     case lltok::kw_sanitize_memory:
     case lltok::kw_sanitize_thread:

llvm/lib/AsmParser/LLToken.h

enum Kind {
   kw_noduplicate,
   kw_noimplicitfloat,
   kw_noinline,
   kw_norecurse,
   kw_nonlazybind,
   kw_nonnull,
   kw_noredzone,
   kw_noreturn,
+  kw_no_simplify_cfg,
   kw_nounwind,
   kw_optnone,
   kw_optsize,
   kw_readnone,
   kw_readonly,
   kw_returned,
   kw_returns_twice,
   kw_signext,

llvm/lib/Bitcode/Reader/BitcodeReader.cpp

static uint64_t getRawAttributeMask(Attribute::AttrKind Val) {
   case Attribute::InaccessibleMemOnly:         return 1ULL << 49;
   case Attribute::InaccessibleMemOrArgMemOnly: return 1ULL << 50;
   case Attribute::SwiftSelf:       return 1ULL << 51;
   case Attribute::SwiftError:      return 1ULL << 52;
   case Attribute::WriteOnly:       return 1ULL << 53;
   case Attribute::Speculatable:    return 1ULL << 54;
   case Attribute::StrictFP:        return 1ULL << 55;
   case Attribute::SanitizeHWAddress: return 1ULL << 56;
+  case Attribute::NoSimplifyCFG:   return 1ULL << 57;
   case Attribute::Dereferenceable:
     llvm_unreachable("dereferenceable attribute not supported in raw format");
     break;
   case Attribute::DereferenceableOrNull:
     llvm_unreachable("dereferenceable_or_null attribute not supported in raw "
                      "format");
     break;
   case Attribute::ArgMemOnly:
static Attribute::AttrKind getAttrFromCode(uint64_t Code) {
   case bitc::ATTR_KIND_DEREFERENCEABLE_OR_NULL:
     return Attribute::DereferenceableOrNull;
   case bitc::ATTR_KIND_ALLOC_SIZE:
     return Attribute::AllocSize;
   case bitc::ATTR_KIND_NO_RED_ZONE:
     return Attribute::NoRedZone;
   case bitc::ATTR_KIND_NO_RETURN:
     return Attribute::NoReturn;
+  case bitc::ATTR_KIND_NO_SIMPLIFY_CFG:
+    return Attribute::NoSimplifyCFG;
   case bitc::ATTR_KIND_NO_UNWIND:
     return Attribute::NoUnwind;
   case bitc::ATTR_KIND_OPTIMIZE_FOR_SIZE:
     return Attribute::OptimizeForSize;
   case bitc::ATTR_KIND_OPTIMIZE_NONE:
     return Attribute::OptimizeNone;
   case bitc::ATTR_KIND_READ_NONE:
     return Attribute::ReadNone;

llvm/lib/Bitcode/Writer/BitcodeWriter.cpp

static uint64_t getAttrKindEncoding(Attribute::AttrKind Kind) {
   case Attribute::Dereferenceable:
     return bitc::ATTR_KIND_DEREFERENCEABLE;
   case Attribute::DereferenceableOrNull:
     return bitc::ATTR_KIND_DEREFERENCEABLE_OR_NULL;
   case Attribute::NoRedZone:
     return bitc::ATTR_KIND_NO_RED_ZONE;
   case Attribute::NoReturn:
     return bitc::ATTR_KIND_NO_RETURN;
+  case Attribute::NoSimplifyCFG:
+    return bitc::ATTR_KIND_NO_SIMPLIFY_CFG;
   case Attribute::NoUnwind:
     return bitc::ATTR_KIND_NO_UNWIND;
   case Attribute::OptimizeForSize:
     return bitc::ATTR_KIND_OPTIMIZE_FOR_SIZE;
   case Attribute::OptimizeNone:
     return bitc::ATTR_KIND_OPTIMIZE_NONE;
   case Attribute::ReadNone:
     return bitc::ATTR_KIND_READ_NONE;

llvm/lib/IR/Attributes.cpp

std::string Attribute::getAsString(bool InAttrGrp) const {
   if (hasAttribute(Attribute::NonNull))
     return "nonnull";
   if (hasAttribute(Attribute::NoRedZone))
     return "noredzone";
   if (hasAttribute(Attribute::NoReturn))
     return "noreturn";
   if (hasAttribute(Attribute::NoRecurse))
     return "norecurse";
+  if (hasAttribute(Attribute::NoSimplifyCFG))
+    return "no_simplify_cfg";
   if (hasAttribute(Attribute::NoUnwind))
     return "nounwind";
   if (hasAttribute(Attribute::OptimizeNone))
     return "optnone";
   if (hasAttribute(Attribute::OptimizeForSize))
     return "optsize";
   if (hasAttribute(Attribute::ReadNone))
     return "readnone";

llvm/lib/IR/Verifier.cpp

static bool isFuncOnlyAttr(Attribute::AttrKind Kind) {
   case Attribute::NoInline:
   case Attribute::AlwaysInline:
   case Attribute::OptimizeForSize:
   case Attribute::StackProtect:
   case Attribute::StackProtectReq:
   case Attribute::StackProtectStrong:
   case Attribute::SafeStack:
   case Attribute::NoRedZone:
+  case Attribute::NoSimplifyCFG:
   case Attribute::NoImplicitFloat:
   case Attribute::Naked:
   case Attribute::InlineHint:
   case Attribute::StackAlignment:
   case Attribute::UWTable:
   case Attribute::NonLazyBind:
   case Attribute::ReturnsTwice:
   case Attribute::SanitizeAddress:

llvm/lib/Transforms/IPO/ForceFunctionAttrs.cpp

return StringSwitch<Attribute::AttrKind>(Kind)
       .Case("nobuiltin", Attribute::NoBuiltin)
       .Case("noduplicate", Attribute::NoDuplicate)
       .Case("noimplicitfloat", Attribute::NoImplicitFloat)
       .Case("noinline", Attribute::NoInline)
       .Case("nonlazybind", Attribute::NonLazyBind)
       .Case("noredzone", Attribute::NoRedZone)
       .Case("noreturn", Attribute::NoReturn)
       .Case("norecurse", Attribute::NoRecurse)
+      .Case("no_simplify_cfg", Attribute::NoSimplifyCFG)
       .Case("nounwind", Attribute::NoUnwind)
       .Case("optnone", Attribute::OptimizeNone)
       .Case("optsize", Attribute::OptimizeForSize)
       .Case("readnone", Attribute::ReadNone)
       .Case("readonly", Attribute::ReadOnly)
       .Case("argmemonly", Attribute::ArgMemOnly)
       .Case("returns_twice", Attribute::ReturnsTwice)
       .Case("safestack", Attribute::SafeStack)

llvm/lib/Transforms/Utils/CodeExtractor.cpp

if (Attr.isStringAttribute()) {
       case Attribute::NoRecurse:
       case Attribute::InlineHint:
       case Attribute::MinSize:
       case Attribute::NoDuplicate:
       case Attribute::NoImplicitFloat:
       case Attribute::NoInline:
       case Attribute::NonLazyBind:
       case Attribute::NoRedZone:
+      case Attribute::NoSimplifyCFG:
       case Attribute::NoUnwind:
       case Attribute::OptimizeNone:
       case Attribute::OptimizeForSize:
       case Attribute::SafeStack:
       case Attribute::SanitizeAddress:
       case Attribute::SanitizeMemory:
       case Attribute::SanitizeThread:
       case Attribute::SanitizeHWAddress:

llvm/lib/Transforms/Utils/SimplifyCFG.cpp

bool SimplifyCFGOpt::run(BasicBlock *BB) {
   }
 
   return Changed;
 }
 
 bool llvm::simplifyCFG(BasicBlock *BB, const TargetTransformInfo &TTI,
                        const SimplifyCFGOptions &Options,
                        SmallPtrSetImpl<BasicBlock *> *LoopHeaders) {
+  const Function *Fn = BB->getParent();
+  if (Fn && Fn->hasFnAttribute(Attribute::NoSimplifyCFG))
+    return false;
   return SimplifyCFGOpt(TTI, BB->getModule()->getDataLayout(), LoopHeaders,
                         Options)
       .run(BB);
 }

llvm/test/Transforms/SimplifyCFG/no-simplify-cfg.ll

+; RUN: opt < %s -simplifycfg -S | FileCheck %s
+
+define i32 @foo(i32 %x) no_simplify_cfg {
+entry:
+  %x.addr = alloca i32, align 4
+  store i32 %x, i32* %x.addr, align 4
+  %0 = load i32, i32* %x.addr, align 4
+  %cmp = icmp sgt i32 %0, 16
+  br i1 %cmp, label %land.rhs, label %land.end
+
+land.rhs:
+  %1 = load i32, i32* %x.addr, align 4
+  %cmp1 = icmp slt i32 %1, 32
+  br label %land.end
+
+land.end:
+  %2 = phi i1 [ false, %entry ], [ %cmp1, %land.rhs ]
+  %conv = zext i1 %2 to i32
+  ret i32 %conv
+
+; CHECK-LABEL: define i32 @foo(i32 %x)
+; CHECK: br i1 %cmp, label %land.rhs, label %land.end
+; CHECK-LABEL: land.rhs:
+; CHECK: br label %land.end
+; CHECK-LABEL: land.end:
+; CHECK: phi {{.*}} %entry {{.*}} %land.rhs
+}
+
+define i32 @bar(i32 %x) {
+entry:
+  %x.addr = alloca i32, align 4
+  store i32 %x, i32* %x.addr, align 4
+  %0 = load i32, i32* %x.addr, align 4
+  %cmp = icmp sgt i32 %0, 16
+  br i1 %cmp, label %land.rhs, label %land.end
+
+land.rhs:
+  %1 = load i32, i32* %x.addr, align 4
+  %cmp1 = icmp slt i32 %1, 32
+  br label %land.end
+
+land.end:
+  %2 = phi i1 [ false, %entry ], [ %cmp1, %land.rhs ]
+  %conv = zext i1 %2 to i32
+  ret i32 %conv
+
+; CHECK-LABEL: define i32 @bar(i32 %x)
+; CHECK-NOT: br
+}