[PATCH] D135124: Add f16 nearbyint support.
Matt Arsenault via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Oct 3 21:26:45 PDT 2022
arsenm added inline comments.
================
Comment at: llvm/test/CodeGen/AMDGPU/fnearbyint.ll:7
; so no need to check anything.
+declare half @llvm.nearbyint.f16(half) #0
----------------
Leonc wrote:
> arsenm wrote:
> > Leonc wrote:
> > > arsenm wrote:
> > > > This test is missing checks somehow
> > > Do we want checks for all of the functions? The comment on lines 5-6 suggests the checks were left out intentionally.
> > The comment is nonsense and it's bad practice to have no checks
> I'm happy to add the checks. If I only add them for `nearbyint.f16` people might argue that the new code is inconsistent. If I add them for every function the changes would be out of scope for the task. What's the preferred option here?
Fix the test by adding checks in a pre commit before this patch
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135124/new/
https://reviews.llvm.org/D135124
More information about the llvm-commits
mailing list