Status Update
No update yet.
Comments
bl...@google.com
je...@google.com
One design decision worth elaboring on is whether or not to InstIntrinsicsCall to InstIntrinsics. With InstIntrinsicsCall no longer derived from InstCall, to get rid of the Target parameter, it can never get lowered into a call (at least not a call for which the target is unknown to the backend). In high-level languages, intrinsics look like function calls, but at Subzero's IR level they look more like instructions. Hence dropping the "Call" from the name would make sense. Alternatives discussed with Antonio include:
- Keep the
InstCalltarget operand separate from theSrcsoperand list, so the goal of aligning the source operands of load/store-like intrinsics with those of regular load/store instructions is still achieved. Unfortunately this causes complications for liveness analysis, since the target operand of a regular call can be a function pointer produced by other instructions which need to be considered live. We could teach Subzero to treat the target operand as an additional source operand, but it's a bit risky and might require more than liveness analysis to be updated. - Move the target operand to the last position of
Srcs[]. This works without affecting things like liveness analysis, but is still a bit risky since we must ensure nothing looks for the target operand at index 0. It would also still be an unused undefined value for intrinsics. - Add virtual methods for accessing load/store operands. Currently there are no virtual methods for
Inst, and since we process many thousands of these it could have a performance impact to make virtual calls. Also note it already uses aKindenum to perform its own RTTI.
It's worth noting that to LLVM, intrinsics are definitely considered functions: call instruction. Despite Subzero's design clearly having been inspired by LLVM, it uses a separate class for representing intrinsics. InstIntrinsicCall also takes an Intrinsics::IntrinsicInfo argument at construction, whereas with LLVM that information is part of the call target.
Hence I intend on moving forward with renaming it to InstIntrinsics, and adding some comments to clarify that it represents a extension instruction which few parts of the compiler have to know the exact functionality about.
hm...@google.com
Branch: master
commit 33a77f7f129311dcbb7e1480af812de2fe9177db
Author: Nicolas Capens <capn@google.com>
Date: Mon Feb 08 15:04:38 2021
Rename InstIntrinsicCall to InstIntrinsic
It is no longer derived from InstCall, and doesn't take a Target
parameter which could be a symbol to a function which implements
the intrinsic.
Note one can still emit actual Call instructions if a function call
is needed. Since the previous change which removed the Target parameter
we can no longer decide between implementing an intrinsic as inline
instructions or a function call at the Subzero level, but we can still
do that at the Reactor level which has its own concept of intrinsics.
This change also removes mentions of intrinsics representing function
calls. It also removes code related to PNaCl-specific LLVM intrinsics,
including the ability to look up intrinsics by name. The addArg(),
getArg(), and getNumArgs() methods, adopted from InstCall (but no longer
inherited from it), are kept for now due to risk of replacing the ones
for InstCall objects, while the confusion caused by keeping the
function-related "arg" term is deemed low.
Bug:
Change-Id: I293f039853abff6f5bebda1b714774205bdec846
Reviewed-on:
Presubmit-Ready: Nicolas Capens <nicolascapens@google.com>
Tested-by: Nicolas Capens <nicolascapens@google.com>
Kokoro-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
M src/Reactor/Optimizer.cpp
M src/Reactor/SubzeroReactor.cpp
M third_party/subzero/docs/DESIGN.rst
M third_party/subzero/src/IceCfg.cpp
M third_party/subzero/src/IceConverter.cpp
M third_party/subzero/src/IceELFObjectWriter.cpp
M third_party/subzero/src/IceGlobalContext.cpp
M third_party/subzero/src/IceGlobalContext.h
M third_party/subzero/src/IceGlobalInits.cpp
M third_party/subzero/src/IceGlobalInits.h
M third_party/subzero/src/IceInst.cpp
M third_party/subzero/src/IceInst.h
M third_party/subzero/src/IceInstrumentation.cpp
M third_party/subzero/src/IceInstrumentation.h
M third_party/subzero/src/IceIntrinsics.cpp
M third_party/subzero/src/IceIntrinsics.h
M third_party/subzero/src/IceTargetLowering.cpp
M third_party/subzero/src/IceTargetLowering.h
M third_party/subzero/src/IceTargetLoweringARM32.cpp
M third_party/subzero/src/IceTargetLoweringARM32.h
M third_party/subzero/src/IceTargetLoweringMIPS32.cpp
M third_party/subzero/src/IceTargetLoweringMIPS32.h
M third_party/subzero/src/IceTargetLoweringX86Base.h
M third_party/subzero/src/IceTargetLoweringX86BaseImpl.h
M third_party/subzero/src/PNaClTranslator.cpp
M third_party/subzero/src/WasmTranslator.cpp
Description
In my project I had created a KMP shared module. In one of the source files in the KMP module, I referenced one of the AndroidX resource annotations (@XmlRes). These are in a KMP-ready library, so the IDE correctly offered to add a dependency on it.
However, when I did that, it ended up just appending this to the bottom of the shared/build.gradle.kts file:
(and sync fails: "Unresolved reference: implementation")
The correct place to add it is higher up in the file: