[PATCH] D47592: [AArch64] Corrected FP16 Intrinsic range checks in Clang + added Sema tests
Sjoerd Meijer via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Jun 4 09:07:03 PDT 2018
SjoerdMeijer accepted this revision.
SjoerdMeijer added a comment.
This revision is now accepted and ready to land.
I think this looks ok now, just some nits inline.
Can you please upload your diffs with more context next time?
================
Comment at: utils/TableGen/NeonEmitter.cpp:2166
+void NeonEmitter::genIntrinsicRangeCheckCode(raw_ostream &OS,
SmallVectorImpl<Intrinsic *> &Defs) {
OS << "#ifdef GET_NEON_IMMEDIATE_CHECK\n";
----------------
Nit: can you realign this?
================
Comment at: utils/TableGen/NeonEmitter.cpp:2193
+ if (Def->getBaseType().getElementSizeInBits() == 16 ||
+ Def->getName().find('h') != std::string::npos)
+ // VCVTh operating on FP16 intrinsics in range [1, 16)
----------------
Nit: for a moment I thought this could match more cases than intended, but we have already checked for isVCVT_N, so should be fine?
================
Comment at: utils/TableGen/NeonEmitter.cpp:2194
+ Def->getName().find('h') != std::string::npos)
+ // VCVTh operating on FP16 intrinsics in range [1, 16)
+ UpperBound = "15";
----------------
Nit: think you're (almost) repeating the comment above, so you can omit this one?
https://reviews.llvm.org/D47592
More information about the cfe-commits
mailing list