cameron.mcinally (Cameron McInally)
User

Projects

User does not belong to any projects.

User Details

User Since
Jan 6 2015, 6:21 AM (201 w, 4 d)

Recent Activity

Yesterday

cameron.mcinally added a comment to D53157: Teach the IRBuilder about constrained fadd and friends.

Fair enough. Just for conversation's sake, I was envisioning the FE support a little differently...

Fri, Nov 16, 3:17 PM
cameron.mcinally abandoned D54121: [FPEnv] Add constrained FCMP intrinsic.

Abandon this Revision for D54649...

Fri, Nov 16, 1:56 PM
cameron.mcinally added inline comments to D54649: [FPEnv] Rough out constrained FCmp intrinsics.
Fri, Nov 16, 1:50 PM
cameron.mcinally created D54649: [FPEnv] Rough out constrained FCmp intrinsics.
Fri, Nov 16, 1:36 PM

Thu, Nov 15

cameron.mcinally added a comment to D54121: [FPEnv] Add constrained FCMP intrinsic.

Oh, right. I missed that. So there should probably be a new STRICT_SETCC node. Thanks.

Thu, Nov 15, 2:28 PM
cameron.mcinally added a comment to D54121: [FPEnv] Add constrained FCMP intrinsic.

I'm working on the changes that we discussed, but they're pretty invasive. A prospective patch is coming soon, but I wanted us to start thinking about how we'll handle these intrinsics at the SelectionDAG level. There are no CMP ISD nodes (also, what does legalization look like??), so this will likely be a significant change.

Thu, Nov 15, 2:09 PM
cameron.mcinally updated the diff for D54549: [FNeg] Add FNeg Instruction to LangRef document.

Good catch, @arsenm. Thanks.

Thu, Nov 15, 7:02 AM

Wed, Nov 14

cameron.mcinally created D54549: [FNeg] Add FNeg Instruction to LangRef document.
Wed, Nov 14, 3:25 PM
cameron.mcinally added a comment to D53877: [IR] Strawman for dedicated FNeg IR instruction.

I just realized the LangRef changes are missing

Wed, Nov 14, 9:57 AM

Tue, Nov 13

cameron.mcinally added a comment to D53877: [IR] Strawman for dedicated FNeg IR instruction.

I just realized the LangRef changes are missing

Tue, Nov 13, 10:59 AM
cameron.mcinally added a comment to D53877: [IR] Strawman for dedicated FNeg IR instruction.

@majnemer has approved the changes offline. Will commit this patch imminently...

Tue, Nov 13, 10:04 AM

Mon, Nov 12

cameron.mcinally added a comment to D53877: [IR] Strawman for dedicated FNeg IR instruction.

David also gave a LGTM for the call-and-invoke-with-ranges.ll change off-line. If he's okay with the function code number change in LLVMBitCodes.h, I'll go ahead and commit this patch.

Mon, Nov 12, 9:23 AM
cameron.mcinally updated the diff for D53877: [IR] Strawman for dedicated FNeg IR instruction.

Rebase and create new function code number, as suggested by @majnemer.

Mon, Nov 12, 9:19 AM

Fri, Nov 9

cameron.mcinally added a comment to D54121: [FPEnv] Add constrained FCMP intrinsic.

Thinking aloud...

Fri, Nov 9, 10:06 AM
cameron.mcinally added a comment to D54121: [FPEnv] Add constrained FCMP intrinsic.

Here's a first whack at a list of operations needed internally:

Fri, Nov 9, 9:26 AM

Thu, Nov 8

cameron.mcinally updated the diff for D53877: [IR] Strawman for dedicated FNeg IR instruction.

Update test/Transforms/MergeFunc/call-and-invoke-with-ranges.ll to account for changes in the Instruction.def enum values.

Thu, Nov 8, 2:30 PM
cameron.mcinally added a comment to D53157: Teach the IRBuilder about constrained fadd and friends.
In 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.

Thu, Nov 8, 11:41 AM
cameron.mcinally added a comment to D54121: [FPEnv] Add constrained FCMP intrinsic.

The X86 builtin story is weird. There should be 9 builtins. I'm not sure how you found 12.

Thu, Nov 8, 9:32 AM
cameron.mcinally added a comment to D53877: [IR] Strawman for dedicated FNeg IR instruction.

Wow, that's great. Thanks, Matt!

Thu, Nov 8, 8:17 AM
cameron.mcinally added a comment to D54121: [FPEnv] Add constrained FCMP intrinsic.

More insight...

Thu, Nov 8, 8:01 AM
cameron.mcinally added a comment to D54121: [FPEnv] Add constrained FCMP intrinsic.
compareSignalingGreaterEqual(a,b) is equivalent to compareSignalingLessUnordered(b, a)

Is it? If a or b is NaN the first one will return false but the second will return true.

Thu, Nov 8, 7:21 AM

Wed, Nov 7

cameron.mcinally added a comment to D54121: [FPEnv] Add constrained FCMP intrinsic.

That's cool with me. Definitely a ton of work, but it's clean and complete.

Wed, Nov 7, 3:54 PM
cameron.mcinally updated the diff for D53877: [IR] Strawman for dedicated FNeg IR instruction.

Add bitcode tests for fastmath flags, as @arsenm suggested. Thanks for bearing with me, Matt.

Wed, Nov 7, 3:24 PM
cameron.mcinally updated the diff for D53877: [IR] Strawman for dedicated FNeg IR instruction.

Add two test/Bitcode tests and address one llvm-bcanalyzer issue, as suggested by @arsenm.

Wed, Nov 7, 2:43 PM
cameron.mcinally added a comment to D53877: [IR] Strawman for dedicated FNeg IR instruction.

Ah, I think I see some problems with llvm-bcanalyzer. Will address...

Wed, Nov 7, 2:14 PM
cameron.mcinally added a comment to D53877: [IR] Strawman for dedicated FNeg IR instruction.

Bitcode compatibility tests are missing?

Wed, Nov 7, 2:06 PM
cameron.mcinally updated the diff for D53877: [IR] Strawman for dedicated FNeg IR instruction.

Add newlines to lib/AsmParser/LLParser.cpp, as suggested by @arsenm.

Wed, Nov 7, 1:24 PM
cameron.mcinally added a comment to D53157: Teach the IRBuilder about constrained fadd and friends.

Seems fair. I don't know enough about the Clang FE to have an opinion on that.

Wed, Nov 7, 8:33 AM
cameron.mcinally updated subscribers of D53157: Teach the IRBuilder about constrained fadd and friends.

Adding the usual suspects: @andrew.w.kaylor @craig.topper @uweigand @hfinkel

Wed, Nov 7, 8:22 AM
Herald added a reviewer for D52839: Inform AST's UnaryOperator of FENV_ACCESS: shafik.
Wed, Nov 7, 8:06 AM

Tue, Nov 6

cameron.mcinally added a comment to D54121: [FPEnv] Add constrained FCMP intrinsic.

All good points.

Tue, Nov 6, 7:18 AM

Mon, Nov 5

cameron.mcinally added inline comments to D54121: [FPEnv] Add constrained FCMP intrinsic.
Mon, Nov 5, 2:06 PM
cameron.mcinally added inline comments to D54121: [FPEnv] Add constrained FCMP intrinsic.
Mon, Nov 5, 1:51 PM
cameron.mcinally created D54121: [FPEnv] Add constrained FCMP intrinsic.
Mon, Nov 5, 1:40 PM
cameron.mcinally added inline comments to D43515: More math intrinsics for conservative math handling.
Mon, Nov 5, 11:51 AM

Fri, Nov 2

cameron.mcinally added a comment to D43142: Experimental pass to convert all floating point operations to the equivalent constrained intrinsics.

We still need to control code motion. For instance, if the code checks the status bits before calling the FENV_ACCESS OFF function we can't have FP operations that are inlined hoisted above the status bit check.

Fri, Nov 2, 2:51 PM
cameron.mcinally added a comment to D43142: Experimental pass to convert all floating point operations to the equivalent constrained intrinsics.

Well, yes and no. In a region defined as FENV_ACCESS off the user has granted the compiler explicit permission to ignore FP status bits and exception behavior. Depending on how we optimize these regions the FP status bits may come out differently and exceptions may or may not be raised, but the user has promised not to unmask exceptions or look at status bits in that region. So if that part of the behavior of the code changes depending on whether or not we inline that's OK because that's what the user signed up for. In that sense the behavior hasn't changed any more than it does between -O0 and -O3.

Fri, Nov 2, 12:28 PM
cameron.mcinally added a comment to D43142: Experimental pass to convert all floating point operations to the equivalent constrained intrinsics.

The fact that we have chosen to inline the function doesn't change the semantics.

Fri, Nov 2, 11:49 AM
cameron.mcinally added a comment to D43142: Experimental pass to convert all floating point operations to the equivalent constrained intrinsics.
In D43142#1285692, @kpn wrote:

Hmmm. A clang option to turn on FENV_ACCESS for entire compilations would work around this issue. So the behavior you are describing here would work for me.

Fri, Nov 2, 10:57 AM
cameron.mcinally added a comment to D43142: Experimental pass to convert all floating point operations to the equivalent constrained intrinsics.
In D43142#1285565, @kpn wrote:

If all optimizations including constant folding, or at least optimizations on floating point, are delayed until after inlining then there's no problem.

Fri, Nov 2, 10:08 AM
cameron.mcinally updated the diff for D53411: [FPEnv] Add constrained CEIL/FLOOR/ROUND/TRUNC intrinsics.

Rebase and remove extra '^' characters from docs/LangRef.rst.

Fri, Nov 2, 9:49 AM
cameron.mcinally updated the diff for D53877: [IR] Strawman for dedicated FNeg IR instruction.

Add a comment about not reordering the llvm-c/Core.h enum values, as @jyknight suggested.

Fri, Nov 2, 8:16 AM

Thu, Nov 1

cameron.mcinally added a comment to D43142: Experimental pass to convert all floating point operations to the equivalent constrained intrinsics.

I thought what we said was that if constrained FP intrinsics were used anywhere in a function then they had to be used everywhere in the function. So if you inline a function with FENV_ACCESS=ON into a function with FENV_ACCESS=OFF then the FP operations in the destination function need to be converted to constrained intrinsics, and if you inline a function with FENV_ACCESS=OFF into a function with FENV_ACCESS=ON then the FP operations in the function being inlined need to be converted. This conversion will have a detrimental effect on optimizations, so it would be nice to factor that into the inling cost, but at this point I'm mostly concerned about generating correct code.

Thu, Nov 1, 8:11 PM
cameron.mcinally added inline comments to D53877: [IR] Strawman for dedicated FNeg IR instruction.
Thu, Nov 1, 1:31 PM
cameron.mcinally updated the diff for D53877: [IR] Strawman for dedicated FNeg IR instruction.

Don't reorder the members in llvm-c/Core.h as suggested by @jyknight.

Thu, Nov 1, 12:05 PM
cameron.mcinally added a comment to D43142: Experimental pass to convert all floating point operations to the equivalent constrained intrinsics.
In D43142#1284296, @kpn wrote:

Then don't we have to deal with this sometime? If #pragma FENV_ACCESS ON is set for the main() example above then is constant folding allowed?

Thu, Nov 1, 12:01 PM
cameron.mcinally added inline comments to D53877: [IR] Strawman for dedicated FNeg IR instruction.
Thu, Nov 1, 11:52 AM
cameron.mcinally added a comment to D43142: Experimental pass to convert all floating point operations to the equivalent constrained intrinsics.
In D43142#1284190, @kpn wrote:

And couldn't we turn off constant folding in the IRBuilder when necessary?

Thu, Nov 1, 11:24 AM
cameron.mcinally added a comment to D43142: Experimental pass to convert all floating point operations to the equivalent constrained intrinsics.

Here's a quick example. You can see in the IR that the ConstantFolder picks up the FDiv as undefined:

Thu, Nov 1, 10:44 AM
cameron.mcinally added a comment to D43142: Experimental pass to convert all floating point operations to the equivalent constrained intrinsics.

I don't think this will work. The ConstantFolder could fold away traps early in the IRBuilder. We also wouldn't catch the FNeg/FSub case either.

Thu, Nov 1, 10:34 AM
cameron.mcinally updated the diff for D53877: [IR] Strawman for dedicated FNeg IR instruction.

Remove extra whitespace from test/Assembler/fast-math-flags.ll.

Thu, Nov 1, 10:26 AM
cameron.mcinally updated the diff for D53981: Fix whitespace in test/Assembler/fast-math-flags.ll .

Add -strict-whitespace option.

Thu, Nov 1, 9:50 AM
cameron.mcinally added inline comments to D53877: [IR] Strawman for dedicated FNeg IR instruction.
Thu, Nov 1, 9:24 AM
cameron.mcinally created D53981: Fix whitespace in test/Assembler/fast-math-flags.ll .
Thu, Nov 1, 9:24 AM
cameron.mcinally updated the diff for D53877: [IR] Strawman for dedicated FNeg IR instruction.

Reorder LLVMAddrSpaceCast as @arsenm suggested.

Thu, Nov 1, 8:39 AM
cameron.mcinally added inline comments to D53877: [IR] Strawman for dedicated FNeg IR instruction.
Thu, Nov 1, 8:14 AM
cameron.mcinally updated subscribers of D53877: [IR] Strawman for dedicated FNeg IR instruction.
Thu, Nov 1, 7:23 AM

Wed, Oct 31

cameron.mcinally updated the diff for D53877: [IR] Strawman for dedicated FNeg IR instruction.

Updating diff to add test/Assembler tests. This includes both fastmath flags and double-round-tripping tests.

Wed, Oct 31, 2:04 PM
cameron.mcinally added inline comments to D53877: [IR] Strawman for dedicated FNeg IR instruction.
Wed, Oct 31, 11:31 AM
cameron.mcinally created D53932: [NFCI][FPEnv] Split constrained intrinsic tests.
Wed, Oct 31, 7:17 AM

Tue, Oct 30

cameron.mcinally added inline comments to D53216: [FPEnv] Add constrained intrinsics for MAXNUM and MINNUM.
Tue, Oct 30, 2:05 PM
cameron.mcinally added a comment to D53877: [IR] Strawman for dedicated FNeg IR instruction.

with the intention of adding more Unary operations in the future. E.g. integer Neg (and maybe Abs, Copysign, etc.).

While there are very clear reasons for FNeg to exist, this isn't so true for any of the others listed here..

Tue, Oct 30, 12:24 PM
cameron.mcinally added inline comments to D53877: [IR] Strawman for dedicated FNeg IR instruction.
Tue, Oct 30, 11:16 AM
cameron.mcinally updated subscribers of D53877: [IR] Strawman for dedicated FNeg IR instruction.
Tue, Oct 30, 11:15 AM
cameron.mcinally added inline comments to D53877: [IR] Strawman for dedicated FNeg IR instruction.
Tue, Oct 30, 11:12 AM
cameron.mcinally created D53877: [IR] Strawman for dedicated FNeg IR instruction.
Tue, Oct 30, 11:01 AM

Thu, Oct 25

cameron.mcinally added a comment to D53216: [FPEnv] Add constrained intrinsics for MAXNUM and MINNUM.

Ping.

Thu, Oct 25, 11:25 AM
cameron.mcinally updated the diff for D53650: [FPEnv] Last BinaryOperator::isFNeg(...) to m_FNeg(...) changes.

Whoops, forgot to update. Yeah, your vector test looks good now.

Thu, Oct 25, 8:51 AM
cameron.mcinally added inline comments to D53650: [FPEnv] Last BinaryOperator::isFNeg(...) to m_FNeg(...) changes.
Thu, Oct 25, 8:08 AM
cameron.mcinally updated the diff for D53650: [FPEnv] Last BinaryOperator::isFNeg(...) to m_FNeg(...) changes.

Updating diff based on Sanjay's suggestions. One comment inline...

Thu, Oct 25, 8:05 AM

Wed, Oct 24

cameron.mcinally created D53650: [FPEnv] Last BinaryOperator::isFNeg(...) to m_FNeg(...) changes.
Wed, Oct 24, 8:42 AM

Tue, Oct 23

cameron.mcinally added a comment to D53205: [FPEnv][NFCI] Convert more BinaryOperator::isFNeg(...) to m_FNeg(...).

@spatel notice the one test change. That seems like a good change to me, i.e. replace a constant pool load with undef. But, perhaps the CP load is a canonicalization that I don't know about....

Tue, Oct 23, 1:05 PM
cameron.mcinally updated the diff for D53205: [FPEnv][NFCI] Convert more BinaryOperator::isFNeg(...) to m_FNeg(...).

Rebase, add Sanjay's changes, and replace some more BinaryOperator::isFNeg(...) calls.

Tue, Oct 23, 12:52 PM
cameron.mcinally accepted D53533: [Reassociate] replace fake binop queries with 'match' API.

LGTM

Tue, Oct 23, 8:25 AM

Mon, Oct 22

cameron.mcinally added a comment to D53205: [FPEnv][NFCI] Convert more BinaryOperator::isFNeg(...) to m_FNeg(...).

Ah, yeah. That would work. Thanks, Sanjay.

Mon, Oct 22, 2:47 PM

Fri, Oct 19

cameron.mcinally added a comment to D53205: [FPEnv][NFCI] Convert more BinaryOperator::isFNeg(...) to m_FNeg(...).

Just thinking aloud. I really don't have enough experience with this framework to say for sure...

Fri, Oct 19, 11:32 AM
cameron.mcinally added a comment to D53205: [FPEnv][NFCI] Convert more BinaryOperator::isFNeg(...) to m_FNeg(...).
  1. There are no users of that "IgnoreZeroSign" optional parameter in trunk - just delete it?

    ` -bool BinaryOperator::isFNeg(const Value *V, bool IgnoreZeroSign) { +bool BinaryOperator::isFNeg(const Value *V) {

    `
Fri, Oct 19, 9:59 AM
cameron.mcinally updated the diff for D53411: [FPEnv] Add constrained CEIL/FLOOR/ROUND/TRUNC intrinsics.

Add support for FROUND and FTRUNC too.

Fri, Oct 19, 8:45 AM

Thu, Oct 18

cameron.mcinally created D53411: [FPEnv] Add constrained CEIL/FLOOR/ROUND/TRUNC intrinsics.
Thu, Oct 18, 1:47 PM

Oct 18 2018

cameron.mcinally added inline comments to D53216: [FPEnv] Add constrained intrinsics for MAXNUM and MINNUM.
Oct 18 2018, 11:19 AM

Oct 17 2018

cameron.mcinally added a comment to D53205: [FPEnv][NFCI] Convert more BinaryOperator::isFNeg(...) to m_FNeg(...).

Also, it will be ugly to compensate for this shortcoming at the caller.

Oct 17 2018, 2:06 PM
cameron.mcinally added a comment to D53205: [FPEnv][NFCI] Convert more BinaryOperator::isFNeg(...) to m_FNeg(...).

Ah, yeah, there is a silent regression here. From BinaryOperator::isFNeg(...):

Oct 17 2018, 12:42 PM

Oct 16 2018

cameron.mcinally updated the diff for D53205: [FPEnv][NFCI] Convert more BinaryOperator::isFNeg(...) to m_FNeg(...).

Rebase to pick up Sanjay's changes...

Oct 16 2018, 7:56 AM

Oct 13 2018

cameron.mcinally added a comment to D53205: [FPEnv][NFCI] Convert more BinaryOperator::isFNeg(...) to m_FNeg(...).

I suspect that none of these changes are actually 'NFC'.

Oct 13 2018, 2:19 PM

Oct 12 2018

cameron.mcinally created D53216: [FPEnv] Add constrained intrinsics for MAXNUM and MINNUM.
Oct 12 2018, 1:51 PM
cameron.mcinally added inline comments to D53205: [FPEnv][NFCI] Convert more BinaryOperator::isFNeg(...) to m_FNeg(...).
Oct 12 2018, 9:57 AM
cameron.mcinally created D53205: [FPEnv][NFCI] Convert more BinaryOperator::isFNeg(...) to m_FNeg(...).
Oct 12 2018, 9:54 AM

Oct 9 2018

cameron.mcinally closed D52934: [FPEnv] PatternMatcher support for checking FNEG ignoring signed zeros.

Bah, I forgot the "Differential Revision:" tag. This was committed with:

Oct 9 2018, 2:53 PM
cameron.mcinally updated the diff for D52934: [FPEnv] PatternMatcher support for checking FNEG ignoring signed zeros.

Move "NSZ" to be a suffix.

Oct 9 2018, 1:34 PM
cameron.mcinally added a comment to D52934: [FPEnv] PatternMatcher support for checking FNEG ignoring signed zeros.

NSW is a flag *on an instruction*. It's not a 'fneg operation of NSZ instruction'.
Suffix variants are cleaner IMO. Would be great to stick to them, and migrate the rest.

Oct 9 2018, 11:38 AM
cameron.mcinally updated the diff for D52934: [FPEnv] PatternMatcher support for checking FNEG ignoring signed zeros.

Update 'ignore signed zero' acronym to "NSZ".

Oct 9 2018, 9:36 AM

Oct 8 2018

cameron.mcinally added inline comments to D52934: [FPEnv] PatternMatcher support for checking FNEG ignoring signed zeros.
Oct 8 2018, 7:47 AM

Oct 5 2018

cameron.mcinally updated the diff for D52934: [FPEnv] PatternMatcher support for checking FNEG ignoring signed zeros.

Small update to remove superfluous parentheses.

Oct 5 2018, 11:46 AM
cameron.mcinally added inline comments to D43515: More math intrinsics for conservative math handling.
Oct 5 2018, 8:26 AM
cameron.mcinally added inline comments to D52934: [FPEnv] PatternMatcher support for checking FNEG ignoring signed zeros.
Oct 5 2018, 8:16 AM
cameron.mcinally created D52934: [FPEnv] PatternMatcher support for checking FNEG ignoring signed zeros.
Oct 5 2018, 8:11 AM

Oct 4 2018

cameron.mcinally added inline comments to D43515: More math intrinsics for conservative math handling.
Oct 4 2018, 11:45 AM

Sep 25 2018

cameron.mcinally added a comment to D50913: [FPEnv] Don't need copysign/fabs/fneg constrained intrinsics.

There hasn't been any strong objects, but I haven't seen many strong accepts either besides the few main stakeholders. I'm under the assumption that silence is a passive reject in situations like this.

Sep 25 2018, 8:09 AM

Sep 21 2018

cameron.mcinally added a comment to D50913: [FPEnv] Don't need copysign/fabs/fneg constrained intrinsics.

It looks like an explicit FNEG operation isn't going to fly. I propose we drop that pursuit and continue forward with a constrained FNEG intrinsic.

Sep 21 2018, 7:43 AM

Aug 24 2018

cameron.mcinally added a comment to D50913: [FPEnv] Don't need copysign/fabs/fneg constrained intrinsics.

Did we decide that fast math flags can't be applied in the presence of constrained operations?

Aug 24 2018, 10:28 AM