Page MenuHomePhabricator

aemerson (Amara Emerson)
Asian George Costanza

Projects

User does not belong to any projects.

User Details

User Since
Sep 9 2013, 3:45 AM (280 w, 2 d)

Compilers at a fruit company

Recent Activity

Yesterday

aemerson added a comment to D56379: GlobalISel: Implement fewerElementsVector for constants.

This should be abandoned?

Tue, Jan 22, 1:21 PM
aemerson accepted D55639: GlobalISel: Allow shift amount to be a different type.

LGTM, thanks.

Tue, Jan 22, 1:19 PM

Fri, Jan 18

aemerson committed rL351617: Revert r351584: "GlobalISel: Verify g_zextload and g_sextload".
Revert r351584: "GlobalISel: Verify g_zextload and g_sextload"
Fri, Jan 18, 4:40 PM

Tue, Jan 15

aemerson accepted D56738: [SelectionDAG] Update check in createOperands to reflect max() is a valid value..

Can any test case be added here, or would it be just too big to realistically test?

I have a file triggering the assert, but I think the cases where it is triggered require 64k load/stores in a single BB. But maybe there is another way to construct a node with more than 64k operands?

Tue, Jan 15, 1:08 PM
aemerson added a comment to D56706: GlobalISel: Combine trunc with constant.

Just checking, this is dependent on D55639?

Tue, Jan 15, 11:35 AM
aemerson accepted D52803: [GISel]: Add support for CSEing continuously during GISel passes.

By default the CSE is enabled during IRTranslator and Legalizer now (disable with -enable-cse-in-legalizer/irtranslator=0).

I naively wonder if unconditionally enabling a CSE optimization even at -O0 is going to lead to a poorer debug experience in some cases?

This is a good point, so I think that if we enable it for -O0 it should only be for constants.

I've also updated most tests (except some tests where it's not obvious what the test is doing where CSE is disabled).

If CSE should be enabled by default, I guess that we'll need to figure out what the tests are supposed to be doing so that there aren't many tests mainly testing non-default behaviour?

Tue, Jan 15, 11:16 AM

Mon, Jan 14

aemerson committed rCTE351100: Revert r351051 "[clangd] Unlink VFS working dir from OS working dir.".
Revert r351051 "[clangd] Unlink VFS working dir from OS working dir."
Mon, Jan 14, 11:03 AM
aemerson committed rL351100: Revert r351051 "[clangd] Unlink VFS working dir from OS working dir.".
Revert r351051 "[clangd] Unlink VFS working dir from OS working dir."
Mon, Jan 14, 11:03 AM
aemerson committed rL351091: Revert "[VFS] Allow multiple RealFileSystem instances with independent CWDs.".
Revert "[VFS] Allow multiple RealFileSystem instances with independent CWDs."
Mon, Jan 14, 10:36 AM

Thu, Jan 10

aemerson accepted D56310: GlobalISel: Implement widen for extract_vector_elt elt type.

LGTM.

Thu, Jan 10, 1:40 PM
aemerson added a comment to D55639: GlobalISel: Allow shift amount to be a different type.

Thanks, this definitely needed to be done. Looks mostly fine, but I also can't be confident on the X86 changes. @igorb @aivchenk can you take a look at this change?

Do you have an opinion how how to handle selecting the shift amount type in the combiner?

For AArch64 we want shift immediates to be 64 bit in order to select using the imported patterns. I can see 3 options:
a) we ask the target via a hook what the preferred shift immediate type is
b) we iteratively check from i64->i32(maybe -> i16?) using the isInstUnsupported() call to see which type combination is legal and pick the first
c) we don't worry about the type and get the legalizer to re-legalize it. I don't think however that the legalizer artifact combiner currently tries to re-legalize the combined instructions. @aditya_nandakumar do you have an opinion?

For now, my slight preference would be option b) as its simpler and doesn't try to change the way the legalizer/combiner currently works.

This does assume a power of 2 legality

Thu, Jan 10, 1:30 PM

Tue, Jan 8

aemerson added a comment to D55639: GlobalISel: Allow shift amount to be a different type.

Thanks, this definitely needed to be done. Looks mostly fine, but I also can't be confident on the X86 changes. @igorb @aivchenk can you take a look at this change?

Do you have an opinion how how to handle selecting the shift amount type in the combiner?

Tue, Jan 8, 1:40 PM

Mon, Jan 7

aemerson added a comment to D55639: GlobalISel: Allow shift amount to be a different type.

Thanks, this definitely needed to be done. Looks mostly fine, but I also can't be confident on the X86 changes. @igorb @aivchenk can you take a look at this change?

Mon, Jan 7, 2:56 PM
aemerson accepted D56378: GlobalISel: Implement fewerElements for implicit_def.

LGTM.

Mon, Jan 7, 1:50 PM
aemerson added a comment to D56379: GlobalISel: Implement fewerElementsVector for constants.

This is strange. G_CONSTANT isn't supposed to be able to splat the constant into a vector, so this is an oversight in the MachineVerifier for not catching this case.

Mon, Jan 7, 1:42 PM
aemerson accepted D56266: [GlobalISel] Fix choice of instruction selector for AArch64 at -O0 with -global-isel=0.
Mon, Jan 7, 1:24 PM
aemerson added a comment to D56266: [GlobalISel] Fix choice of instruction selector for AArch64 at -O0 with -global-isel=0.

Thanks for the cleanup. LGTM with minor naming change.

Mon, Jan 7, 1:21 PM
aemerson added inline comments to D55814: GlobalISel: Support narrowing zextload/sextload.
Mon, Jan 7, 1:18 PM
aemerson added inline comments to D56310: GlobalISel: Implement widen for extract_vector_elt elt type.
Mon, Jan 7, 1:15 PM

Dec 20 2018

aemerson accepted D55970: [GlobalISel][AArch64] Add support for widening G_FCEIL.

LGTM with minor change.

Dec 20 2018, 5:00 PM
aemerson added a reviewer for D55639: GlobalISel: Allow shift amount to be a different type: aemerson.
Dec 20 2018, 3:15 PM

Dec 19 2018

aemerson committed rL349723: Fix build errors introduced by r349712 on aarch64 bots..
Fix build errors introduced by r349712 on aarch64 bots.
Dec 19 2018, 7:31 PM
aemerson committed rL349712: [AArch64][GlobalISel] Implement selection og G_MERGE of two s32s into s64..
[AArch64][GlobalISel] Implement selection og G_MERGE of two s32s into s64.
Dec 19 2018, 5:14 PM
aemerson accepted D55863: [GlobalISel][AArch64] Add support for @llvm.ceil.

LGTM. In general for IR translator tests we don't need to check for every type combination if it's a simple 1-1 mapping, but no need to change the patch.

Dec 19 2018, 10:01 AM

Dec 13 2018

aemerson added a comment to D51362: [GlobalISel][IRTranslator] Canonicalize G_ICMP to have constant operands last.

I'd like to revive this patch, as the other IR canonicalization pass is much more controversial.

Dec 13 2018, 2:00 PM

Dec 10 2018

aemerson closed D53629: [GlobalISel] Restrict G_MERGE_VALUES capability and replace with new opcodes.

Thanks, committed in r348788.

Dec 10 2018, 10:52 AM
aemerson committed rL348788: [GlobalISel] Restrict G_MERGE_VALUES capability and replace with new opcodes..
[GlobalISel] Restrict G_MERGE_VALUES capability and replace with new opcodes.
Dec 10 2018, 10:48 AM

Dec 7 2018

aemerson added a comment to D55392: [GlobalISel] Add IR translation support for the @llvm.log10 intrinsic.

The patch title should probably say adding IR translation support instead of selection support. If we're adding selection support too then we should have a test case, otherwise it can be in a later patch.

Dec 7 2018, 11:35 AM
aemerson accepted D55392: [GlobalISel] Add IR translation support for the @llvm.log10 intrinsic.

LGTM.

Dec 7 2018, 10:40 AM
aemerson accepted D55418: [GlobalISel] Set stack protector index when translating Intrinsic::stackprotector.
Dec 7 2018, 9:44 AM

Dec 5 2018

aemerson committed rL348430: [GlobalISel] Introduce G_BUILD_VECTOR, G_BUILD_VECTOR_TRUNC and G_CONCAT_VECTOR….
[GlobalISel] Introduce G_BUILD_VECTOR, G_BUILD_VECTOR_TRUNC and G_CONCAT_VECTOR…
Dec 5 2018, 3:57 PM
aemerson closed D53594: [GlobalISel] Introduce G_BUILD_VECTOR and G_CONCAT_VECTOR opcodes.
Dec 5 2018, 3:57 PM
aemerson added a comment to D54710: [x86] don't try to convert add with undef operands to LEA.
This seems to me to be a bug in ProcessImplicitDefs, for some reasoning killing the IMPLICIT_DEF even though there's a subreg use. GlobalISel MIR output looks fine to me.

Regardless of whether there's a bug in ProcessImplicitDefs, we would have to correctly propagate the undef flag through this transform (from 'add' to 'lea') though? I'm proposing to avoid that for this case based on existing code that appears to do the same thing.

Dec 5 2018, 9:33 AM

Dec 4 2018

aemerson committed rL348324: [SelectionDAG] Split very large token factors for loads into 64k chunks..
[SelectionDAG] Split very large token factors for loads into 64k chunks.
Dec 4 2018, 4:46 PM
aemerson closed D55073: [SelectionDAG] Split very large token factors for loads into 64k chunks.
Dec 4 2018, 4:46 PM
aemerson added inline comments to D55073: [SelectionDAG] Split very large token factors for loads into 64k chunks.
Dec 4 2018, 4:33 PM
aemerson committed rL348320: [AArch64][GlobalISel] Re-enable selection of volatile loads..
[AArch64][GlobalISel] Re-enable selection of volatile loads.
Dec 4 2018, 4:06 PM
aemerson added a comment to D54710: [x86] don't try to convert add with undef operands to LEA.

This seems to me to be a bug in ProcessImplicitDefs, for some reasoning killing the IMPLICIT_DEF even though there's a subreg use. GlobalISel MIR output looks fine to me.

Dec 4 2018, 4:03 PM

Dec 3 2018

aemerson updated the diff for D53629: [GlobalISel] Restrict G_MERGE_VALUES capability and replace with new opcodes.

Updated to incorporate Volkan's patches to change buildMerge calls to explicitly check for types before choosing the right build method. Also added test for and fixed artifact combiner for G_BUILD_VECTOR.

Dec 3 2018, 4:36 PM

Nov 30 2018

aemerson updated the diff for D53629: [GlobalISel] Restrict G_MERGE_VALUES capability and replace with new opcodes.

Addressed comments, deleted the non power of 2 test.

Nov 30 2018, 4:28 PM
aemerson updated the diff for D55073: [SelectionDAG] Split very large token factors for loads into 64k chunks.

Changed the approach to replace values in PendingLoads in place. Also added an assert in SelectionDAG::createOperands() to check if we can store the requested number of operands.

Nov 30 2018, 1:31 PM
aemerson added a comment to D55073: [SelectionDAG] Split very large token factors for loads into 64k chunks.

Does this fix PR7250 and PR37000 ? Those test cases look usable.

Nov 30 2018, 1:22 PM

Nov 29 2018

aemerson added inline comments to D55073: [SelectionDAG] Split very large token factors for loads into 64k chunks.
Nov 29 2018, 2:52 PM
aemerson created D55073: [SelectionDAG] Split very large token factors for loads into 64k chunks.
Nov 29 2018, 1:07 PM

Nov 28 2018

aemerson added inline comments to D53629: [GlobalISel] Restrict G_MERGE_VALUES capability and replace with new opcodes.
Nov 28 2018, 5:10 PM
aemerson accepted D54174: [GlobalISel] LegalizationArtifactCombiner: Combine aext([asz]ext x) -> [asz]ext x.
Nov 28 2018, 4:22 PM
aemerson accepted D54518: [GlobalISel] Fix insertion of stack-protector epilogue.

Hi Petr,

Nov 28 2018, 3:04 PM
aemerson added inline comments to D53629: [GlobalISel] Restrict G_MERGE_VALUES capability and replace with new opcodes.
Nov 28 2018, 2:57 PM

Nov 15 2018

aemerson added a comment to D54174: [GlobalISel] LegalizationArtifactCombiner: Combine aext([asz]ext x) -> [asz]ext x.

So these extends are created by the legalizer, correct? If so, it seems ok to me as that's what these artifact combines were intended for.

Nov 15 2018, 7:14 AM

Nov 5 2018

aemerson updated the diff for D53594: [GlobalISel] Introduce G_BUILD_VECTOR and G_CONCAT_VECTOR opcodes.

Added comment for G_BUILD_VECTOR_TRUNC and tests for the verifier. I haven't added all the different combinations because we can only write one test per file, so I tested the most useful of the verifier errors.

Nov 5 2018, 4:38 PM
aemerson added inline comments to D53594: [GlobalISel] Introduce G_BUILD_VECTOR and G_CONCAT_VECTOR opcodes.
Nov 5 2018, 9:56 AM
aemerson updated the diff for D53594: [GlobalISel] Introduce G_BUILD_VECTOR and G_CONCAT_VECTOR opcodes.

Updated patch to introduce a separate G_BUILD_VECTOR_TRUNC opcode.

Nov 5 2018, 9:43 AM

Nov 2 2018

aemerson added a comment to D53594: [GlobalISel] Introduce G_BUILD_VECTOR and G_CONCAT_VECTOR opcodes.

I'm not sure how this is any better. If we had an opcode G_BUILD_VECTOR_TRUNC, it would have a superset of the functionality of the G_BUILD_VECTOR. You'd still have this powerful opcode, except now you have potential code duplication in handling the two, as well as a hindered G_BUILD_VECTOR.

Unless you're say that G_BUILD_VECTOR _TRUNC can *only* deal with oversize scalars, in which case it's an odd opcode for a corner case (and I would suggest here that we just deal with the inconvenient types, but I'm not married to the idea).

I was suggesting the later.

Generally speaking, I am not a fan of implicit stuff, thus the proposal (and that's easy to have a isBuildVectorLike or something). That said, I'd like to go back as why we need to do that.

So the problem was that we wanted to represent:
a(<4 x s32>) = G_BUILD_VECTOR p(s32), q, r, s
b(<4 x s8>) = G_TRUNC a

into
b(< 4 x s8>) = G_BUILD_VECTOR p(s32), q, r, s

Why can't we keep the "verbose" representation?
I would expect it would be easy to handle that in ISel.

As long as <4 x s32> = the G_BUILD_VECTOR could be legalized I think the worst that will happen is that we just get poor codegen.

Also, just throwing that here, I haven't really thought about it: should we rethink our legality model to check patterns instead of one instruction at a time?
I.e., maybe

a(s8) = ...

is not legal but

a(s8) = ...
b(<4 x s8>) = build_vector a(s8)

is.

I think the issue of sequences becoming illegal because of things like copies being introduced is still a deal breaker for that, and it would also introduce a code motion barrier since we can't select sequences spanning blocks. E.g. it wouldn't be safe to move the insn in the predecessor due to other uses.

This actually ties back to one question that I don't think we answered. Do we legalize on the type (i.e., s8 to s32) or the size and type can be different (i.e., s8(8b) to s8(32b))?
It seems to me we are trying to write <4 x s8> = s8(32b), ...., but maybe we want <4 x s8> = s32, ....

I've had a think about what having separate type and container sizes would actually mean. For brevity, let’s call values which have distinct type and container sizes “partial values”. A partial value s8:s32 means that the upper 24 bits of this 32 bit register is always undef. For some operations, it should always be possible to ignore the type size and just use the container size for selection. E.g. non-flag setting G_ADD could select a 32 bit add and satisfy the semantics of an s8 add. But for other operations like shifts, or count-leading-zeros, we can't use a wider operation type, so the entire chain of partial value operations would be restricted to selecting the narrower type, rather than the container. And at that point, the container type may as well not be there. There may be a way around this issue I haven't seen though.

Anyhow, to be concrete, I am fine with the implicit truncate, we know this is not too bad based on SDISel experience, I want to make sure we consider all our options here.

Sure. Given that we already have this implicit truncating behavior on account of G_MERGE being an omnipotent operation, should we allow the implicit truncation with G_BUILD_VECTOR for now? If we have problems with it later we can revisit the idea of adding a separate opcode/something else.

Cheers,
-Quentin

Nov 2 2018, 8:19 AM

Oct 30 2018

aemerson added a comment to D53594: [GlobalISel] Introduce G_BUILD_VECTOR and G_CONCAT_VECTOR opcodes.

Thanks @aditya_nandakumar for the context.

I understand where you're coming from.

What I meant is that I would rather have one opcode of build_vector and one for build_vector with trunc. The rationale is that it makes it clear what you want to achieve and it makes for a stronger type verifier.

Oct 30 2018, 4:10 PM
aemerson accepted D53740: [globalisel][irtranslator] Verify that DILocations aren't lost in translation.

LGTM with the nits addressed.

Oct 30 2018, 9:47 AM

Oct 29 2018

aemerson updated the diff for D53629: [GlobalISel] Restrict G_MERGE_VALUES capability and replace with new opcodes.

Updated AArch64 legalization. I've removed the unsupported case since we're actually making it valid.

Oct 29 2018, 9:38 AM
aemerson added inline comments to D53740: [globalisel][irtranslator] Verify that DILocations aren't lost in translation.
Oct 29 2018, 9:32 AM
aemerson updated the diff for D53594: [GlobalISel] Introduce G_BUILD_VECTOR and G_CONCAT_VECTOR opcodes.

Updated patch to allow G_BUILD_VECTOR to have scalar operands not identical to the result vector element types. This allows implicit truncation.

Oct 29 2018, 9:29 AM
aemerson added inline comments to D53740: [globalisel][irtranslator] Verify that DILocations aren't lost in translation.
Oct 29 2018, 6:32 AM

Oct 26 2018

aemerson added inline comments to D53629: [GlobalISel] Restrict G_MERGE_VALUES capability and replace with new opcodes.
Oct 26 2018, 7:13 AM

Oct 25 2018

aemerson closed D53592: [GlobalISel] Use the target preferred type for G_EXTRACT_VECTOR_ELT index.

Committed in r345265.

Oct 25 2018, 7:09 AM
aemerson committed rL345265: [GlobalISel] Use the target preferred type for G_EXTRACT_VECTOR_ELT index..
[GlobalISel] Use the target preferred type for G_EXTRACT_VECTOR_ELT index.
Oct 25 2018, 7:07 AM

Oct 24 2018

aemerson updated subscribers of D53594: [GlobalISel] Introduce G_BUILD_VECTOR and G_CONCAT_VECTOR opcodes.

+ @huntergr On a tangential note, G_BUILD_VECTOR for expressing broadcasts of scalars won't work for SVE and other variable vector width targets. We do however still leave the door open here for a new G_BROADCAST opcode in future.

Oct 24 2018, 8:01 AM
aemerson added a comment to D53594: [GlobalISel] Introduce G_BUILD_VECTOR and G_CONCAT_VECTOR opcodes.

Are two different opcodes for this really needed? Why not a single opcode with uniformly typed operands, either scalar or vector, which concatenates the operands as an appropriately sized vector? I know there's precedent for the separation elsewhere in LLVM, but it has always seemed redundant to me personally. The code for handling the BUILD vs. CONCAT cases tends to be very similar.

Oct 24 2018, 7:52 AM

Oct 23 2018

aemerson added a parent revision for D53629: [GlobalISel] Restrict G_MERGE_VALUES capability and replace with new opcodes: D53594: [GlobalISel] Introduce G_BUILD_VECTOR and G_CONCAT_VECTOR opcodes.
Oct 23 2018, 6:10 PM
aemerson added a child revision for D53594: [GlobalISel] Introduce G_BUILD_VECTOR and G_CONCAT_VECTOR opcodes: D53629: [GlobalISel] Restrict G_MERGE_VALUES capability and replace with new opcodes.
Oct 23 2018, 6:10 PM
aemerson created D53629: [GlobalISel] Restrict G_MERGE_VALUES capability and replace with new opcodes.
Oct 23 2018, 6:10 PM
aemerson added a parent revision for D53594: [GlobalISel] Introduce G_BUILD_VECTOR and G_CONCAT_VECTOR opcodes: D53592: [GlobalISel] Use the target preferred type for G_EXTRACT_VECTOR_ELT index.
Oct 23 2018, 11:29 AM
aemerson added a child revision for D53592: [GlobalISel] Use the target preferred type for G_EXTRACT_VECTOR_ELT index: D53594: [GlobalISel] Introduce G_BUILD_VECTOR and G_CONCAT_VECTOR opcodes.
Oct 23 2018, 11:29 AM
aemerson created D53594: [GlobalISel] Introduce G_BUILD_VECTOR and G_CONCAT_VECTOR opcodes.
Oct 23 2018, 11:27 AM
aemerson created D53592: [GlobalISel] Use the target preferred type for G_EXTRACT_VECTOR_ELT index.
Oct 23 2018, 11:18 AM

Oct 15 2018

aemerson added a comment to D52803: [GISel]: Add support for CSEing continuously during GISel passes.

Thanks for working on this. Apart from Daniel's suggestion, I have a few more comments but haven't done an in depth analysis of the memory costs/algorithmic properties.

Oct 15 2018, 11:20 AM

Oct 11 2018

aemerson committed rL344251: [InstCombine] Fix SimplifyLibCalls erasing an instruction while IC still had….
[InstCombine] Fix SimplifyLibCalls erasing an instruction while IC still had…
Oct 11 2018, 7:53 AM
aemerson closed D52729: [InstCombine] Fix SimplifyLibCalls erasing an instruction while IC still had references to it.
Oct 11 2018, 7:53 AM
aemerson updated the diff for D52729: [InstCombine] Fix SimplifyLibCalls erasing an instruction while IC still had references to it.
Oct 11 2018, 7:45 AM
aemerson added inline comments to D52729: [InstCombine] Fix SimplifyLibCalls erasing an instruction while IC still had references to it.
Oct 11 2018, 7:34 AM

Oct 9 2018

aemerson added a comment to D52729: [InstCombine] Fix SimplifyLibCalls erasing an instruction while IC still had references to it.

Ping.

Oct 9 2018, 4:52 AM

Oct 1 2018

aemerson created D52729: [InstCombine] Fix SimplifyLibCalls erasing an instruction while IC still had references to it.
Oct 1 2018, 9:17 AM

Sep 20 2018

aemerson requested review of D51953: [GlobalISel] Add a new IR canonicalization pass.
Sep 20 2018, 5:58 AM
aemerson updated the diff for D51953: [GlobalISel] Add a new IR canonicalization pass.

@aditya_nandakumar I've added a target hook that defaults to enabling this.

Sep 20 2018, 5:58 AM
aemerson accepted D48575: [GISel]; Add a helper legalization rule for legalizing addressspacecast if no-op.

This seems like something that most targets would want done by default. I wonder if we should have a standard set of legalizer actions that targets can use. LGTM anyway.

Sep 20 2018, 5:52 AM

Sep 17 2018

aemerson added a comment to D51953: [GlobalISel] Add a new IR canonicalization pass.

Hi Amara - would it be possible to add this as part of AArch64 pass pipeline? Adding this to all targets would result in needless burning of compile time for targets that don't need this.

Sep 17 2018, 3:53 PM
aemerson added a comment to D51953: [GlobalISel] Add a new IR canonicalization pass.

This isn't really ideal because of how trivial it is currently. The issue is that if we added the capability to InstSimplify (which is where it would fit most naturally), we'd still end up with the problem of running the whole of InstSimplify in the GlobalISel pipeline. It's reasonable for now I think so I'll commit with the changes requested.

Sep 17 2018, 9:24 AM
aemerson added a comment to D52131: [GISel][NFC]: Make MachineIRBuilder fully stateless.

Looks ok, if a bit cumbersome with the additional state (do we need to hold references to it?). I'd like to see how this state is used with the CSE builder first though.

Sep 17 2018, 9:18 AM
aemerson committed rL342397: Revert "Revert r342183 "[DAGCombine] Fix crash when store merging created an….
Revert "Revert r342183 "[DAGCombine] Fix crash when store merging created an…
Sep 17 2018, 7:43 AM

Sep 13 2018

aemerson committed rL342183: [DAGCombine] Fix crash when store merging created an extract_subvector with….
[DAGCombine] Fix crash when store merging created an extract_subvector with…
Sep 13 2018, 2:30 PM
aemerson closed D51831: [DAGCombine] Fix crash when store merging created an extract_subvector with invalid index.
Sep 13 2018, 2:30 PM
aemerson added a comment to D51831: [DAGCombine] Fix crash when store merging created an extract_subvector with invalid index.

Ok, I'm surprised that would be a realistic scenario but I'll make the change. Thanks.

Sep 13 2018, 11:02 AM

Sep 12 2018

aemerson updated the diff for D51831: [DAGCombine] Fix crash when store merging created an extract_subvector with invalid index.

You're right, that's a simplified expression.

Sep 12 2018, 3:38 PM
aemerson added a comment to D51831: [DAGCombine] Fix crash when store merging created an extract_subvector with invalid index.

Ping.

Sep 12 2018, 7:13 AM
aemerson added a comment to D51953: [GlobalISel] Add a new IR canonicalization pass.

Hi Quentin,

Sep 12 2018, 4:14 AM

Sep 11 2018

aemerson created D51953: [GlobalISel] Add a new IR canonicalization pass.
Sep 11 2018, 3:02 PM

Sep 8 2018

aemerson created D51831: [DAGCombine] Fix crash when store merging created an extract_subvector with invalid index.
Sep 8 2018, 6:32 AM

Sep 5 2018

aemerson added a comment to D51145: Guard FMF context by excluding some FP operators from FPMathOperator.

I added one more non fmf instruction and there is room to add others if needed.

There's also discussion about the definition of FPMathOperator and the relation to FMF here:
https://bugs.llvm.org/show_bug.cgi?id=38086
D51646

Sep 5 2018, 5:11 PM
aemerson added inline comments to D51646: DAG: Preserve FMF when creating fminnum/fmaxnum.
Sep 5 2018, 5:08 PM

Sep 4 2018

aemerson added a comment to D51145: Guard FMF context by excluding some FP operators from FPMathOperator.

Ok, to me it seems like instructions for which fast math flags can't apply shouldn't be classed as FPMathOperators. insertelement/extractelement are just moving raw bits. If we disallowed those instruction types in the FPMathOperator classof() then this problem should go away, and we don't incorrectly relax FP semantics like in the case I pointed out.

Sep 4 2018, 6:22 AM

Aug 31 2018

aemerson added a comment to D51145: Guard FMF context by excluding some FP operators from FPMathOperator.

I’m not seeing how FP instructions can not carry flags. The IR semantics are defined to be strict FP unless explicitly relaxed.

Aug 31 2018, 12:35 PM
aemerson updated subscribers of D51145: Guard FMF context by excluding some FP operators from FPMathOperator.
Aug 31 2018, 12:17 PM
aemerson added a comment to D51362: [GlobalISel][IRTranslator] Canonicalize G_ICMP to have constant operands last.

It can't do that all the time, with these G_ICMPs for example it also has to swap the predicate (not that we even have imported patterns that could possibly match for AArch64).

Why not?
I understand G_ICMPs are special, but we could come up with whatever complicated logic in TableGen.

Aug 31 2018, 12:00 PM
aemerson added a comment to D51145: Guard FMF context by excluding some FP operators from FPMathOperator.

Hit submit too early:
So with the above code B will not have the flag cleared? If that's the case it doesn't seem right to me.

Aug 31 2018, 11:45 AM