[PATCH] D115924: [ConstantFolding] Unify handling of load from uniform value

cameron.mcinally added inline comments. Comment at: llvm/lib/Analysis/ConstantFolding.cpp:722 + (Ty->isIntOrIntVectorTy() || Ty->isFPOrFPVectorTy())) +return Constant::getAllOnesValue(Ty); + return nullptr; nikic wrote: > cameron.mcinally wrote: > > Sor

[PATCH] D115924: [ConstantFolding] Unify handling of load from uniform value

cameron.mcinally added inline comments. Herald added a project: All. Comment at: llvm/lib/Analysis/ConstantFolding.cpp:722 + (Ty->isIntOrIntVectorTy() || Ty->isFPOrFPVectorTy())) +return Constant::getAllOnesValue(Ty); + return nullptr; Sorry for the lat

[PATCH] D117795: [AArch64] Fixes for strict FP vector instructions

cameron.mcinally added a comment. Is it possible to break the 4 subtasks into separate reviews? -In clang generate fcmps when appropriate for neon intrinsics -Fix legalization of scalarized strict FP vector operations -Add some missing strict FP handling to AArch64TargetLowering -Adjust t

[PATCH] D71767: [POC][SVE] Allow code generation for fixed length vectorised loops [Patch 2/2].

cameron.mcinally added a comment. MIN/MAX and DIVs are up my alley. I'll try those. Will check out VREDUCE if I get through the others. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71767/new/ https://reviews.llvm.org/D71767 _

[PATCH] D71767: [POC][SVE] Allow code generation for fixed length vectorised loops [Patch 2/2].

cameron.mcinally added a comment. In D71767#2206947 , @paulwalker-arm wrote: > @cameron.mcinally this is the patch I mentioned the other day, which contains > the nodes where once I've written suitable tests I'll push separate patches > for. Thanks, Pa

[PATCH] D76949: Replace subtract-from-zero float in version with fneg in PowerPC special fma compiler builtins

cameron.mcinally accepted this revision. cameron.mcinally added a comment. This revision is now accepted and ready to land. LGTM, assuming Harbormaster whines are addressed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76949/new/ https://reviews.l

[PATCH] D76688: [AArch64][SVE] Add SVE intrinsics for masked loads & stores

cameron.mcinally accepted this revision. cameron.mcinally added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76688/new/ https://reviews.llvm.org/D76688 ___

[PATCH] D75467: [instcombine] remove fsub to fneg hacks; only emit fneg

cameron.mcinally accepted this revision. cameron.mcinally added a comment. This revision is now accepted and ready to land. LGTM. Thanks, Simon. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75467/new/ https://reviews.llvm.org/D75467 ___

[PATCH] D75467: [instcombine] remove fsub to fneg hacks; only emit fneg

cameron.mcinally added a comment. Sorry, forgot to submit my nit... Comment at: llvm/include/llvm/IR/PatternMatch.h:75 +/// Match an arbitrary binary operation and ignore it. +inline class_match m_UnOp() { + return class_match(); Nit: Comment needs updating.

[PATCH] D75130: Remove BinaryOperator::CreateFNeg

cameron.mcinally accepted this revision. cameron.mcinally added a comment. This revision is now accepted and ready to land. LGTM, with the TODO to be handled under a separate patch. Thank you for taking care of this, Simon. Comment at: llvm/lib/Transforms/InstCombine/InstCombi

[PATCH] D74550: [AArch64][SVE] Add SVE index intrinsic

cameron.mcinally accepted this revision. cameron.mcinally added a comment. This revision is now accepted and ready to land. Herald added a subscriber: tatyana-krasnukha. LGTM Comment at: llvm/include/llvm/IR/IntrinsicsAArch64.td:804 +[IntrNoMem]>; + class Adv

[PATCH] D69878: Consoldiate internal denormal flushing controls

cameron.mcinally accepted this revision. cameron.mcinally added a comment. LGTM too. Would be good if an expert reviewed the target-specific changes, but they seem safe enough either way. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69878/new/ https://reviews.llvm.org/D69878 __

[PATCH] D72820: [FPEnv] Add pragma FP_CONTRACT support under strict FP.

cameron.mcinally added inline comments. Comment at: clang/lib/CodeGen/CGExprScalar.cpp:3436 +} + } + andrew.w.kaylor wrote: > cameron.mcinally wrote: > > cameron.mcinally wrote: > > > cameron.mcinally wrote: > > > > I don't think it's safe to fuse a FMUL and

[PATCH] D72820: [FPEnv] Add pragma FP_CONTRACT support under strict FP.

cameron.mcinally added inline comments. Comment at: clang/lib/CodeGen/CGExprScalar.cpp:3436 +} + } + cameron.mcinally wrote: > cameron.mcinally wrote: > > I don't think it's safe to fuse a FMUL and FADD if the intermediate > > rounding isn't exactly the sam

[PATCH] D72820: [FPEnv] Add pragma FP_CONTRACT support under strict FP.

cameron.mcinally added inline comments. Comment at: clang/lib/CodeGen/CGExprScalar.cpp:3436 +} + } + cameron.mcinally wrote: > I don't think it's safe to fuse a FMUL and FADD if the intermediate rounding > isn't exactly the same as those individual operatio

[PATCH] D72820: [FPEnv] Add pragma FP_CONTRACT support under strict FP.

cameron.mcinally added inline comments. Comment at: clang/lib/CodeGen/CGExprScalar.cpp:3436 +} + } + I don't think it's safe to fuse a FMUL and FADD if the intermediate rounding isn't exactly the same as those individual operations. FMULADD doesn't guarant

[PATCH] D69878: Consoldiate internal denormal flushing controls

cameron.mcinally added inline comments. Comment at: llvm/docs/LangRef.rst:1837 + are present, this overrides ``"denormal-fp-math"``. Not all targets + support separately setting the denormal mode per type. + arsenm wrote: > andrew.w.kaylor wrote: > > cameron.

[PATCH] D69878: Consoldiate internal denormal flushing controls

cameron.mcinally added inline comments. Comment at: llvm/docs/LangRef.rst:1837 + are present, this overrides ``"denormal-fp-math"``. Not all targets + support separately setting the denormal mode per type. + andrew.w.kaylor wrote: > arsenm wrote: > > andrew.w

[PATCH] D69878: Consoldiate internal denormal flushing controls

cameron.mcinally added a comment. Ok, thanks for the clarifications. Looks good to me, but it would be good to have experts in OpenCL/Cuda/AMDGPU review the target specific changes. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69878/new/ https://reviews.llvm.org/D69878

[PATCH] D69878: Consoldiate internal denormal flushing controls

cameron.mcinally added a comment. This is looking pretty good to me, but I'm ignoring some of the target specific code that I'm not familiar with. Is `denormal-fp-math` influenced by `-Ofast`? Or are there plans for that? Seems like `-Ofast` should imply DAZ and FTZ (if supported by target). I

[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

cameron.mcinally added a comment. In D62731#1779872 , @mibintc wrote: > In D62731#1778597 , > @michele.scandale wrote: > > > >> I've noticed you removed the change for `CompilerInvocation.cpp` about > > >> the ini

[PATCH] D61675: [WIP] Update IRBuilder::CreateFNeg(...) to return a UnaryOperator

cameron.mcinally added a comment. Both the ASan build: http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/35926 and the OCaml build: http://lab.llvm.org:8011/builders/clang-x86_64-debian-fast/builds/19255 have completed successfully with the latest patches. Will continu

[PATCH] D61675: [WIP] Update IRBuilder::CreateFNeg(...) to return a UnaryOperator

cameron.mcinally added a comment. Fix incoming. Sorry for not running the ASan tests... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61675/new/ https://reviews.llvm.org/D61675 ___ cfe-commits mailing

[PATCH] D61675: [WIP] Update IRBuilder::CreateFNeg(...) to return a UnaryOperator

cameron.mcinally added a comment. Apologies, @pcc. Looking now. Will revert if it's not obvious... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61675/new/ https://reviews.llvm.org/D61675 ___ cfe-commi

[PATCH] D61675: [WIP] Update IRBuilder::CreateFNeg(...) to return a UnaryOperator

cameron.mcinally added a comment. Recommitted this patch with Ocaml test fix. I was not able to find quick-start documentation for the Ocaml bindings, so the Ocaml failure has not been tested. That said, the failure mode seems extremely low risk. Will monitor the buildbots for problems... Rep

[PATCH] D61675: [WIP] Update IRBuilder::CreateFNeg(...) to return a UnaryOperator

cameron.mcinally added a subscriber: RKSimon. cameron.mcinally added a comment. @gribozavr I see that you also reverted @RKSimon's commit for the OCaml/core.ml failure: Author: gribozavr Date: Thu Oct 10 07:16:58 2019 New Revision: 374357 URL: http://llvm.org/viewvc/llvm-project?rev=3

[PATCH] D61675: [WIP] Update IRBuilder::CreateFNeg(...) to return a UnaryOperator

cameron.mcinally added a comment. In D61675#1703749 , @cameron.mcinally wrote: > > I reverted it in r374354. Please test before re-landing. > > Hmm, how do I run those tests? I did not see that failure with check-all. > > That's a pretty straightforward f

[PATCH] D61675: [WIP] Update IRBuilder::CreateFNeg(...) to return a UnaryOperator

cameron.mcinally added a comment. > I reverted it in r374354. Please test before re-landing. Hmm, how do I run those tests? I did not see that failure with check-all. That's a pretty straightforward failure. It's just a one-line IR change: `fsub float {{.*}}0{{.*}}, %F1` -> `fneg float %F1` R

[PATCH] D53157: Teach the IRBuilder about constrained fadd and friends

cameron.mcinally added inline comments. Comment at: include/llvm/IR/IRBuilder.h:1244 } /// Copy fast-math-flags from an instruction rather than using the builder's I think you can forgo the `else {` in these functions since the if branch returns immediat

[PATCH] D53157: Teach the IRBuilder about constrained fadd and friends

cameron.mcinally added a comment. In D53157#1316860 , @uweigand wrote: > In my reading of the standard text, there is no special provision for library > code. This means that in general, calling any library function while running > with nondefault trap

[PATCH] D53157: Teach the IRBuilder about constrained fadd and friends

cameron.mcinally added a comment. Digressing a bit, but has anyone given thought to how this implementation will play out with libraries? When running with traps enabled, libraries must be compiled for trap-safety. E.g. optimizations on a lib's code could introduce a NaN or cause a trap that do

[PATCH] D53157: Teach the IRBuilder about constrained fadd and friends

cameron.mcinally added a comment. In https://reviews.llvm.org/D53157#1304900, @andrew.w.kaylor wrote: > I think at this point we're all on the same page in this regard. I just > wanted to make sure we also understand why we're on that page. I still > believe it was the correct choice. Yup, we

[PATCH] D53157: Teach the IRBuilder about constrained fadd and friends

cameron.mcinally added a comment. In https://reviews.llvm.org/D53157#1304880, @hfinkel wrote: > In https://reviews.llvm.org/D53157#1304873, @cameron.mcinally wrote: > > > I'd like to hear more about this. The fesetexcept(...) function and friends > > change the FPEnv state, which can change the

[PATCH] D53157: Teach the IRBuilder about constrained fadd and friends

cameron.mcinally added a comment. In https://reviews.llvm.org/D53157#1304347, @uweigand wrote: > OK, let me try to expand on my point 3 above, which appears to have confused > everybody :-) Ah, that makes more sense. The problem I was missing is when a FENV_ACCESS=OFF operation, like a FDIV,

[PATCH] D53157: Teach the IRBuilder about constrained fadd and friends

cameron.mcinally added a comment. In https://reviews.llvm.org/D53157#1304275, @kpn wrote: > In https://reviews.llvm.org/D53157#1303398, @cameron.mcinally wrote: > > > If we all agree upon that, then we simply have to treat the functions that > > modify the FPEnv, e.g. fesetexcept(...), as barrie

[PATCH] D53157: Teach the IRBuilder about constrained fadd and friends

cameron.mcinally added a comment. In https://reviews.llvm.org/D53157#1302724, @uweigand wrote: > A couple of comments on the previous discussion: > > 1. Instead of defining a new command line option, I'd prefer to use the > existing options -frounding-math and -ftrapping-math to set the default

[PATCH] D53157: Teach the IRBuilder about constrained fadd and friends

cameron.mcinally added a comment. In https://reviews.llvm.org/D53157#1301992, @hfinkel wrote: > > Just because FENV_ACCESS can be toggled on that granularity doesn't mean we > > have to represent it that way. We've previously agreed (I think) that if > > FENV_ACCESS is enabled anywhere in a fun

[PATCH] D53157: Teach the IRBuilder about constrained fadd and friends

cameron.mcinally added a comment. Fair enough. Just for conversation's sake, I was envisioning the FE support a little differently... 1. Add a command line option, say `-ffpe_safe=[true|false]`, that toggles FPEnv-safe code generation for a whole file. A `-ffpe_safe=true` would be equivalent t

[PATCH] D53157: Teach the IRBuilder about constrained fadd and friends

cameron.mcinally added a comment. In https://reviews.llvm.org/D53157#1291964, @kpn wrote: > I don't expect anyone would need to switch between constrained and regular > floating point at an instruction level of granularity. The Standard allows for this (to some degree). E.g. FENV_ACCESS can be