[PATCH] D114778: [XCOFF][FastISel] make fast isel can lower general intrinsics

[ChenZheng via Phabricator via llvm-commits]

shchenz added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/FastISel.cpp:1784
+    // But fast isel still have the ability to do selection for intrinsics.
+    if (TM.getTargetTriple().isOSAIX() && !dyn_cast<IntrinsicInst>(I))
       return false;
----------------
nemanjai wrote:
> qiucf wrote:
> > - In this case, no extra instruction will be really emitted from this intrinsic, but what if it's an intrinsic producing a real function call?
> > - Is it necessary to put the check here, not in `PPCFastISel::fastSelectInstruction`?
> > - In this case, no extra instruction will be really emitted from this intrinsic, but what if it's an intrinsic producing a real function call?
> I don't think this is an issue since `FastISel::selectIntrinsicCall()` only handles a fixed list of intrinsics and the rest are handled in `FastISel::fastLowerIntrinsicCall()` which we don't override.
> > - Is it necessary to put the check here, not in `PPCFastISel::fastSelectInstruction`?
> +1
> The fact that we have the same check in both places seems odd and problematic. And the fact that we are now diverging with those two checks is even stranger.
> 
I deleted the redundant logic in `PPCFastISel::fastSelectInstruction` for `Instruction::Call`. It is exactly the same as the one in the target-independent one. And PowerPC will also use the target-independent ISel.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114778/new/

https://reviews.llvm.org/D114778