[llvm] r230775 - Change the fast-isel-abort option from bool to int to enable "levels"

[Andrea Di Biagio]

Hi Mehdi,

At the moment, if fast-isel aborts and flag fast-isel-abort is set, we
end up calling 'llvm_unreachable'.
Wouldn't it be better to use 'report_fatal_error' instead of calling
'llvm_unreachable'?

Example:

> <snip>...
>    const Function &Fn = *mf.getFunction();
>    MF = &mf;
> @@ -1182,7 +1181,7 @@ void SelectionDAGISel::SelectAllBasicBlo
>          if (!FastIS->lowerArguments()) {
>            // Fast isel failed to lower these arguments
>            ++NumFastIselFailLowerArguments;
> -          if (EnableFastISelAbortArgs)
> +          if (EnableFastISelAbort > 1)
>              llvm_unreachable("FastISel didn't lower all arguments");


If we use 'report_fatal_error', then we avoid to get an 'UNREACHABLE
executed' message if EnableFastISelAbort is > 1.

>
>            // Use SelectionDAG argument lowering
> @@ -1252,6 +1251,10 @@ void SelectionDAGISel::SelectAllBasicBlo
>              dbgs() << "FastISel missed call: ";
>              Inst->dump();
>            }
> +          if (EnableFastISelAbort > 2)
> +            // FastISel selector couldn't handle something and bailed.
> +            // For the purpose of debugging, just abort.
> +            llvm_unreachable("FastISel didn't select the entire block");

Same here. Wouldn't it be better to use 'report_fatal_error' ?

>
>            if (!Inst->getType()->isVoidTy() && !Inst->use_empty()) {
>              unsigned &R = FuncInfo->ValueMap[Inst];
> @@ -1279,24 +1282,21 @@ void SelectionDAGISel::SelectAllBasicBlo
>            continue;
>          }
>
> -        if (isa<TerminatorInst>(Inst) && !isa<BranchInst>(Inst)) {
> -          // Don't abort, and use a different message for terminator misses.
> -          NumFastIselFailures += NumFastIselRemaining;
> -          if (EnableFastISelVerbose || EnableFastISelAbort) {
> +        if (EnableFastISelVerbose || EnableFastISelAbort) {
> +          if (isa<TerminatorInst>(Inst)) {
> +            // Use a different message for terminator misses.
>              dbgs() << "FastISel missed terminator: ";
> -            Inst->dump();
> -          }
> -        } else {
> -          NumFastIselFailures += NumFastIselRemaining;
> -          if (EnableFastISelVerbose || EnableFastISelAbort) {
> +          } else {
>              dbgs() << "FastISel miss: ";
> -            Inst->dump();
>            }
> -          if (EnableFastISelAbort)
> -            // The "fast" selector couldn't handle something and bailed.
> -            // For the purpose of debugging, just abort.
> -            llvm_unreachable("FastISel didn't select the entire block");
> +          Inst->dump();
>          }
> +        if (EnableFastISelAbort > 2)
> +          // FastISel selector couldn't handle something and bailed.
> +          // For the purpose of debugging, just abort.
> +          llvm_unreachable("FastISel didn't select the entire block");
> +
> +        NumFastIselFailures += NumFastIselRemaining;
>          break;
>        }

Same here.

I hope this makes sense.

Cheers,
Andrea