[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