[PATCH] D99976: [WIP] Allow invokable sub-classes of IntrinsicInst

[Philip Reames via Phabricator via llvm-commits]

reames created this revision.
Herald added subscribers: dexonsmith, dantrushin, pengfei, hiraditya, mcrosier.
Herald added a reviewer: bollu.
reames requested review of this revision.
Herald added a project: LLVM.

It used to be that all of our intrinsics were call instructions, but over time, we've added more and more invokable intrinsics.  According to the verifier, we're up to 8 right now.  As IntrinsicInst is a sub-class of CallInst, this puts us in an awkward spot where the idiomatic means to check for intrinsic has a false negative if the intrinsic is invoked.

This change switches IntrinsicInst from being a sub-class of CallInst to being a subclass of CallBase.  This allows invoked intrinsics to be instances of IntrinsicInst, at the cost of requiring a few more casts to CallInst in places where the intrinsic really is known to be a call, not an invoke.

This is admittedly a debatable stylistic issue.  To me, allowing invokeable intrinsics seems cleaner, but I also spent a lot of my time dealing with those intrinsics.  Do others agree?

Do note that the patch as posted is a mess.  There's a bunch of cleanup to happen before actual review.  Here's my punch list for later:

- Add an AssumeInst subclass and retype signatures with it, avoids all of the assumption related changes.
- Kill all calls to IntrinsicInst::Create.  (This actually calls CallInst::Create.)
- Retype params from CallInst to AppropriateIntrinsicInst.
- Remember to build non-x86 targets, there will be changes needed.

With the above addressed, the patch size should drop to near nothing.  This is a hacked up initial patch just to make sure I could get all the tests passing at reasonable investment.

After this lands (if it does), planned cleanups:

- Merge intrinsic handling in InstCombine and use idiomatic visitIntrinsicInst entry point for InstVisitor.
- Do the same in SelectionDAG.
- Do the same in FastISEL.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D99976

Files:
  llvm/include/llvm/Analysis/AssumeBundleQueries.h
  llvm/include/llvm/Analysis/AssumptionCache.h
  llvm/include/llvm/IR/InstVisitor.h
  llvm/include/llvm/IR/IntrinsicInst.h
  llvm/include/llvm/IR/Statepoint.h
  llvm/lib/Analysis/AssumeBundleQueries.cpp
  llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
  llvm/lib/CodeGen/SelectionDAG/StatepointLowering.cpp
  llvm/lib/CodeGen/ShadowStackGCLowering.cpp
  llvm/lib/CodeGen/StackProtector.cpp
  llvm/lib/ExecutionEngine/Interpreter/Execution.cpp
  llvm/lib/Target/X86/X86FastISel.cpp
  llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
  llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp
  llvm/lib/Transforms/InstCombine/InstCombineInternal.h
  llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
  llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
  llvm/unittests/Analysis/AssumeBundleQueriesTest.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D99976.335580.patch
Type: text/x-patch
Size: 24079 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210406/f6d92cfc/attachment.bin>