[PATCH] D71370: [AArch64][SVE] Add integer arithmetic with immediate instructions.
Eli Friedman via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Dec 12 09:18:58 PST 2019
efriedma added inline comments.
================
Comment at: llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp:2828
+ Imm = CurDAG->getTargetConstant(ImmVal & 0xFF, DL, MVT::i32);
+ return true;
+ case MVT::i16:
----------------
dancgr wrote:
> efriedma wrote:
> > I'd prefer to reject immediates greater than 255 here, instead of masking off the bits.
> In the immediate definition we have:
>
> ```
> def addsub_imm8_opt_lsl_i8 : imm8_opt_lsl<8, "uint8_t", SVEAddSubImmOperand8, [{
> return AArch64_AM::isSVEAddSubImm<int8_t>(Imm);
> }]>;
> ```
> This checks if the immediate is in the accepted range for AddSub. Would you say we should also add another check here?
>
> I pasted the code of that function below, since it is not in the changed files from this patch:
>
> ```
> template <typename T>
> static inline bool isSVEAddSubImm(int64_t Imm) {
> bool IsInt8t =
> std::is_same<int8_t, typename std::make_signed<T>::type>::value;
> return uint8_t(Imm) == Imm || (!IsInt8t && uint16_t(Imm & ~0xff) == Imm);
> }
> ```
Not sure that TableGen code is actually getting called from isel in this situation? If it is, fine, we don't need a redundant check.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D71370/new/
https://reviews.llvm.org/D71370
More information about the llvm-commits
mailing list