Options.EnableFastISel is true for targets without FastISel implementation


At -O0, the flag TargetMachine.Options.EnableFastISel is true by default even if the target does not implement FastISel. Unfortunately, some code generation passes check this flag.

One example: With SelectionDAG, the failure basic block for a stack protector check contains a call to library function RTLIB::STACKPROTECTOR_CHECK_FAIL. With FastISel, the call to the library function is hardcoded to __stack_chk_fail (or __stack_smash_handler if on OpenBSD).

If I now call setLibcallName() to set a different function name for the stack protector check fail function, then everything is fine except for -O0 - even if the target does not implement FastISel.

IMHO the flag TargetMachine.Options.EnableFastISel should not be true if the target does not implement FastISel. Or do I overlook something? Any idea how to fix this?


Do you have a testcase? Do you mean this is broken if you try to explicitly enable fast-isel on a target which doesn’t support it?

I expect that Options.EnableFastISel is false by default when the target does not implement FastISel.


target datalayout = "E-m:e-i1:8:16-i8:8:16-i64:64-f128:64-a:8:16-n32:64"
target triple = "s390x-unknown-linux"

define dso_local signext i32 @bug() #0 {
  ret i32 0

attributes #0 = { noinline nounwind optnone sspreq "frame-pointer"="all" "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="z10" }

With llc < bug.ll -stop-after=stack-protector the resulting IR is:

  define dso_local signext i32 @bug() #0 {
    %StackGuardSlot = alloca i8*, align 8
    %0 = call i8* @llvm.stackguard()
    call void @llvm.stackprotector(i8* %0, i8** %StackGuardSlot)
    ret i32 0

In the SelectionDAG implementation of the stack protector feature, the check for the stack failure is inserted during lowering.

But with llc < bug.ll -stop-after=stack-protector -O0 the check gets inserted:

  define dso_local signext i32 @bug() #0 {
    %StackGuardSlot = alloca i8*, align 8
    %0 = call i8* @llvm.stackguard()
    call void @llvm.stackprotector(i8* %0, i8** %StackGuardSlot)
    %1 = call i8* @llvm.stackguard()
    %2 = load volatile i8*, i8** %StackGuardSlot, align 8
    %3 = icmp eq i8* %1, %2
    br i1 %3, label %SP_return, label %CallStackCheckFailBlk, !prof !0

  SP_return:                                        ; preds = %entry
    ret i32 0

  CallStackCheckFailBlk:                            ; preds = %entry
    call void @__stack_chk_fail()

This output does not change when I additionally specify -enable-selectiondag-sp on the command line.

The condition in StackProtector::InsertStackProtectors() is:

  bool SupportsSelectionDAGSP =
      TLI->useStackGuardXorFP() ||
      (EnableSelectionDAGSP && !TM->Options.EnableFastISel);

TLI->useStackGuardXorFP() is false because the SystemZ target does not use it. EnableSelectionDAGSP is true when I specify -enable-selectiondag-sp on the command line. Since the fastisel code is generated, the flag TM->Options.EnableFastISel must be true in this case. I checked this with adding a llvm::dbgs() statement.

I’m not sure if it is a bug. My expectation is that the EnableFastISel flag (and the O0WantsFastISel flag in the TargetMachine class) only takes a true value if the target has a FastISel implementation.

I think you’re right, the logic in TargetPassConfig::addCoreISelPasses looks broken. It’s assuming based on fast-isel should be used at -O0 if the -fast-isel flag is unset, whereas I would assume O0WantsFastIsel would be for the target to control.

However, I think it’s more broken that the StackProtector pass is inspecting the requested selector. On a fallback, IR passes are not re-run. It wouldn’t really matter if the target consistently hit the fallback, but the code here seems to be assuming that will never happen.

However, I think it’s more broken that the StackProtector pass is inspecting the requested selector.

Yes, I agree with this.
My first step to fix this would be to add a flag to TargetMachine, indicating if the target implements FastISel or not. This flag can then be used in TargetPassConfig::addCoreISelPasses() when EnableFastISel and O0WantsFastISel. When this is in place, then the uses of EnableFastISel needs to be evaluated if it’s used under the reight assumptions.

I’m reluctant to add flags when it is likely that the existing flags aren’t being used in a sensible way. In particular my impression was that O0WantsFastISel was the “target supports FastISel” flag. It would seem odd for a target to support FastISel but not want to use it by default even at O0.

This is what I expect, too. The reality from TargetPassConfig is:

  // Enable FastISel with -fast-isel, but allow that to be overridden.
  TM->setO0WantsFastISel(EnableFastISelOption != cl::BOU_FALSE);

Which means that O0WantsFastISel is only false if the user specified -fast-isel=false on the command line. This even holds when the target only implements GlobalISel.

With the additional flag I only want to make sure that the existing flags reflect this properly. E.g.

  TM->setO0WantsFastISel((EnableFastISelOption != cl::BOU_FALSE) &&

Of course, better ideas are welcome.

Ah… now that I’ve done the git-blame and resurrected at least some memory of what I was trying to do 6 years ago when I invented it… seems that O0WantsFastISel is a flag on TM only because it needs to be saved/restored somewhere else (in order to handle optnone in a non-O0 scenario). It was in fact never more than a cached copy of the -fast-isel option.

Well! In that case, add a supportsFastISel with my blessing.