[PATCH] D51933: [AMDGPU] Ensure trig range reduction only used for subtargets that require it
Matt Arsenault via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Sep 13 04:28:42 PDT 2018
arsenm added inline comments.
================
Comment at: lib/Target/AMDGPU/SIISelLowering.cpp:6655
+ SDValue MulVal = DAG.getNode(ISD::FMUL, DL, VT, Arg,
+ DAG.getConstantFP(0.5 / M_PI, DL, VT));
+ TrigVal = DAG.getNode(AMDGPUISD::FRACT, DL, VT, MulVal);
----------------
dstuttard wrote:
> arsenm wrote:
> > dstuttard wrote:
> > > arsenm wrote:
> > > > Avoid repeating the same constant.
> > > >
> > > > Also we should stop relying on system headers for these constants
> > > Re system headers - why, are you concerned that M_PI may differ between compilers?
> > > Can we defer removing reliance on system headers to a separate change that does this globally?
> > Yes. AFAIK the standard doesn't actually specify the exact rounded value. For other system provided constants, LLVM usually duplicates the definitions and puts them in Support
> That's a reasonable point.
>
> However, would you mind if I defer that to a different change as making changes in Support will spill this change over into a core change - the old code used M_PI before, so it's no worse as it stands.
Yes. It also doesn't need to be in support. We already locally defined in for MSVC
Repository:
rL LLVM
https://reviews.llvm.org/D51933
More information about the llvm-commits
mailing list