Page MenuHomePhabricator

volkan (Volkan Keles)
User

Projects

User does not belong to any projects.

User Details

User Since
Jun 30 2015, 12:54 PM (216 w, 1 d)

Recent Activity

Tue, Aug 20

volkan added a comment to D61787: [GlobalISel Legalizer] Improve artifact combiner.

Hi Petar,

Tue, Aug 20, 3:28 PM · Restricted Project

Thu, Aug 15

volkan committed rG0ae6006bee0e: [GlobalISel] CSEMIRBuilder: Add support for G_GEP (authored by volkan).
[GlobalISel] CSEMIRBuilder: Add support for G_GEP
Thu, Aug 15, 4:46 PM
volkan committed rL369070: [GlobalISel] CSEMIRBuilder: Add support for G_GEP.
[GlobalISel] CSEMIRBuilder: Add support for G_GEP
Thu, Aug 15, 4:45 PM
volkan closed D66316: [GlobalISel] CSEMIRBuilder: Add support for G_GEP.
Thu, Aug 15, 4:45 PM · Restricted Project
volkan updated the summary of D66316: [GlobalISel] CSEMIRBuilder: Add support for G_GEP.
Thu, Aug 15, 3:41 PM · Restricted Project
volkan created D66316: [GlobalISel] CSEMIRBuilder: Add support for G_GEP.
Thu, Aug 15, 3:15 PM · Restricted Project
volkan abandoned D66315: [GlobalISel] CSEMIRBuilder: Add support for G_GEP.
Thu, Aug 15, 3:11 PM · Restricted Project
volkan created D66315: [GlobalISel] CSEMIRBuilder: Add support for G_GEP.
Thu, Aug 15, 3:11 PM · Restricted Project

Mon, Aug 12

volkan accepted D66039: [GlobalISel]: Fix a bug in KnownBits where we should have been using SizeInBits.

LGTM.

Mon, Aug 12, 1:55 PM · Restricted Project

Fri, Aug 9

volkan added a comment to D61787: [GlobalISel Legalizer] Improve artifact combiner.

In this case, moving %1 to AuxiliaryArtifactList doesn't make any difference because combining %9 will remove it anyway, even if it was in InstList. Since you already check the source registers before legalizing the artifacts, this shouldn't be a problem. What do you think?

Well in this example it is not a problem, but in more complicated case:
moving %1 to AuxiliaryArtifactList means that %1 will be legalized first if %9 failed to combine it away, %9 might be able to combine away the artifact that was produced during legalization of %1.

Fri, Aug 9, 11:52 AM · Restricted Project

Thu, Aug 8

volkan added a comment to D61787: [GlobalISel Legalizer] Improve artifact combiner.

@volkan Regarding D65894.
Here, artifacts that failed to combine are moved to RetryList and we retry to combine them. Once all of the MachineInstrs that define use operands of our artifact are processed (are not in any of the Observer Lists) we turn artifact into an instruction. This way we have more opportunities for combines. e.g. in /test/AMDGPU/GlobalISel/legalize-unmerge-values.mir , this patch catches G_MERGE/G_UNMERGE combine that D65894 cannot catch because those two are declared legal.
As for the other test changes in D65894, all of them are essentially here.
I also tried test/CodeGen/AArch64/GlobalISel/retry-artifact-combine.mir and got similar output, but with one less copy instr.
there are differences in test/CodeGen/AMDGPU/GlobalISel/legalize-{xor|and|or}.mir
D65894 managed to combine few more G_TRUNC + G_ANYEXT but in the end both patches crash on %0:_(<4 x s8>) = G_TRUNC %29:_(<4 x s32>), because order of combine attempts is different this patch crashed before attempting to combine mentioned G_TRUNC + G_ANYEXT.
Please check if I missed something the this patch does not cover compared to D65894.

See D65199 for more tests with output from -debug option.

Thu, Aug 8, 9:08 PM · Restricted Project

Wed, Aug 7

volkan created D65894: [GlobalISel] Legalizer: Retry combining illegal artifacts as long as there new artifacts.
Wed, Aug 7, 10:52 AM · Restricted Project

Jun 19 2019

volkan committed rG61d7e35b22bd: Fix GlobalISel MachineVerifier tests. NFC. (authored by volkan).
Fix GlobalISel MachineVerifier tests. NFC.
Jun 19 2019, 11:16 AM
volkan committed rL363854: Fix GlobalISel MachineVerifier tests. NFC..
Fix GlobalISel MachineVerifier tests. NFC.
Jun 19 2019, 11:13 AM

Jun 17 2019

volkan committed rG689509edab4d: [test][AArch64] Relax the check line for G_BRJT in legalizer-info-validation.mir (authored by volkan).
[test][AArch64] Relax the check line for G_BRJT in legalizer-info-validation.mir
Jun 17 2019, 2:23 PM
volkan committed rL363621: [test][AArch64] Relax the check line for G_BRJT in legalizer-info-validation.mir.
[test][AArch64] Relax the check line for G_BRJT in legalizer-info-validation.mir
Jun 17 2019, 2:23 PM
volkan accepted D63405: GlobalISel: Don't lose fneg flags when lowering to fsub.

I think we should preserve the existing flags and be consistent with what we do in SDAGISel. LGTM.

Jun 17 2019, 11:23 AM

Jun 7 2019

volkan committed rG97204a6788a5: [GlobalISel] IRTranslator: Translate the intrinsics ignored by CodeGen (authored by volkan).
[GlobalISel] IRTranslator: Translate the intrinsics ignored by CodeGen
Jun 7 2019, 1:17 PM
volkan committed rL362834: [GlobalISel] IRTranslator: Translate the intrinsics ignored by CodeGen.
[GlobalISel] IRTranslator: Translate the intrinsics ignored by CodeGen
Jun 7 2019, 1:17 PM
volkan closed D63022: [GlobalISel] IRTranslator: Translate the intrinsics ignored by CodeGen.
Jun 7 2019, 1:17 PM · Restricted Project
volkan updated the diff for D63022: [GlobalISel] IRTranslator: Translate the intrinsics ignored by CodeGen.
Jun 7 2019, 1:04 PM · Restricted Project
volkan updated the diff for D63022: [GlobalISel] IRTranslator: Translate the intrinsics ignored by CodeGen.

Merged the switch cases.

Jun 7 2019, 12:55 PM · Restricted Project
volkan updated the summary of D63022: [GlobalISel] IRTranslator: Translate the intrinsics ignored by CodeGen.
Jun 7 2019, 12:46 PM · Restricted Project
volkan created D63022: [GlobalISel] IRTranslator: Translate the intrinsics ignored by CodeGen.
Jun 7 2019, 12:44 PM · Restricted Project

Apr 23 2019

volkan accepted D60973: [llvm-extract] Expose the group extraction feature of the BlockExtractor.

Thanks Quentin, LGTM.

Apr 23 2019, 11:25 AM · Restricted Project
volkan accepted D60971: [BlockExtractor] Expose a constructor for the group extraction.

LGTM

Apr 23 2019, 11:24 AM · Restricted Project
volkan accepted D60970: [BlockExtractor] Change the basic block separator from ',' to ';'.

LGTM

Apr 23 2019, 11:20 AM · Restricted Project

Apr 18 2019

volkan accepted D60746: [BlockExtractor] Extend the file format to support the grouping of basic blocks.

LGTM, thanks Quentin. Could you please update the summary before pushing this?

Apr 18 2019, 10:14 AM · Restricted Project

Apr 16 2019

volkan added a comment to D60746: [BlockExtractor] Extend the file format to support the grouping of basic blocks.

Hi Quentin,

Apr 16 2019, 11:15 AM · Restricted Project

Mar 11 2019

volkan accepted D59227: [GlobalISel][Utils] Teach getConstantVRegVal how to look through trunc and z|sext.

LGTM.

Mar 11 2019, 1:23 PM · Restricted Project

Feb 12 2019

volkan added inline comments to D54468: [LoadStoreVectorizer] Fix infinite loop in reorder..
Feb 12 2019, 10:53 AM · Restricted Project

Jan 29 2019

volkan accepted D57109: GlobalISel: Implement fewerElementsVector for select.

LGTM with a few nits.

Jan 29 2019, 7:50 PM
volkan added a comment to D56969: GlobalISel: Try to make legalize rules more useful for vectors.

My preference here would be to have a separate set of mutations for vector elements, @dsanders @aditya_nandakumar what do you think?

What about having 3 versions for each of these? e.g. minScalar, minElement, minScalarOrElement?

Jan 29 2019, 4:37 PM

Jan 28 2019

volkan accepted D57360: [GlobalISel] Add IRTranslator support for @llvm.sqrt -> G_FSQRT.

LGTM

Jan 28 2019, 2:42 PM
volkan accepted D57359: [GlobalISel] Introduce a G_FSQRT generic instruction.

LGTM

Jan 28 2019, 2:42 PM
volkan added a reviewer for D57360: [GlobalISel] Add IRTranslator support for @llvm.sqrt -> G_FSQRT: volkan.
Jan 28 2019, 2:42 PM
volkan added a reviewer for D57359: [GlobalISel] Introduce a G_FSQRT generic instruction: volkan.
Jan 28 2019, 2:42 PM

Jan 23 2019

volkan added a comment to D57117: GlobalISel: Add helper to LLT to get a scalar or vector.

Could you add a test to unittests/CodeGen/LowLevelTypeTest.cpp?

Jan 23 2019, 1:47 PM

Jan 19 2019

volkan accepted D56956: GlobalISel: Add isPointer legality predicates.

LGTM.

Jan 19 2019, 2:47 PM

Jan 18 2019

volkan accepted D56862: GlobalISel: Implement widenScalar for basic FP ops.

LGTM.

Jan 18 2019, 11:34 AM

Jan 17 2019

volkan added a reviewer for D56835: GlobalISel: Make buildConstant handle vectors: volkan.
Jan 17 2019, 2:22 PM
volkan added inline comments to D56863: GlobalISel: Implement fewerElementsVector for basic FP ops.
Jan 17 2019, 12:02 PM
volkan accepted D56863: GlobalISel: Implement fewerElementsVector for basic FP ops.

LGTM.

Jan 17 2019, 12:02 PM
volkan added a reviewer for D56863: GlobalISel: Implement fewerElementsVector for basic FP ops: volkan.
Jan 17 2019, 12:02 PM

Dec 14 2018

volkan committed rL349200: [GlobalISel] LegalizerHelper: Implement fewerElementsVector for G_LOAD/G_STORE.
[GlobalISel] LegalizerHelper: Implement fewerElementsVector for G_LOAD/G_STORE
Dec 14 2018, 2:14 PM
volkan closed D53728: [GlobalISel] LegalizerHelper: Implement fewerElementsVector for G_LOAD/G_STORE.
Dec 14 2018, 2:14 PM
volkan updated the diff for D53728: [GlobalISel] LegalizerHelper: Implement fewerElementsVector for G_LOAD/G_STORE.
  • Replaced buildMerge with buildBuildVector/buildConcatVectors.
  • Updated the test to avoid generating G_CONCAT_VECTORS as it's not legal for v4s32/v2s64.
Dec 14 2018, 2:05 PM

Dec 10 2018

volkan added inline comments to D55294: [GISel]: Refactor MachineIRBuilder to allow passing additional parameters to build Instrs.
Dec 10 2018, 12:48 PM
volkan accepted D53629: [GlobalISel] Restrict G_MERGE_VALUES capability and replace with new opcodes.

Thanks Amara, LGTM.

Dec 10 2018, 9:48 AM

Dec 5 2018

volkan accepted D54980: [NFC][GISel]: Provide an standard interface to observe changes across GISel passes.

LGTM.

Dec 5 2018, 11:31 AM

Nov 29 2018

volkan added inline comments to D53629: [GlobalISel] Restrict G_MERGE_VALUES capability and replace with new opcodes.
Nov 29 2018, 3:02 PM
volkan committed rL347893: [GlobalISel] LegalizationArtifactCombiner: Combine aext([asz]ext x) -> [asz]ext….
[GlobalISel] LegalizationArtifactCombiner: Combine aext([asz]ext x) -> [asz]ext…
Nov 29 2018, 10:22 AM
volkan closed D54174: [GlobalISel] LegalizationArtifactCombiner: Combine aext([asz]ext x) -> [asz]ext x.
Nov 29 2018, 10:22 AM

Nov 27 2018

volkan added a comment to D53629: [GlobalISel] Restrict G_MERGE_VALUES capability and replace with new opcodes.

Hi Volkan,

We are unable to legalize the test below because of the missing combine.

This sentence jumped at me. Are you saying we have mandatory combines at this point for legalization to work?
Although that wouldn't surprise me, I would like we have a clear list of what they are (if that's not already the case.)

Cheers,
-Quentin

Nov 27 2018, 9:47 AM

Nov 26 2018

volkan added inline comments to D53629: [GlobalISel] Restrict G_MERGE_VALUES capability and replace with new opcodes.
Nov 26 2018, 2:53 PM
volkan 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 26 2018, 9:00 AM
volkan added a reviewer for D54518: [GlobalISel] Fix insertion of stack-protector epilogue: aemerson.
Nov 26 2018, 8:58 AM

Nov 13 2018

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

What's the reason for not putting this in a post-legalize combiner? It seems to me that we'll have to implement this in the Legalizer and the Combiner

Nov 13 2018, 2:29 AM

Nov 7 2018

volkan added a comment to D53629: [GlobalISel] Restrict G_MERGE_VALUES capability and replace with new opcodes.

Hi Amara,

Nov 7 2018, 10:44 AM

Nov 6 2018

volkan added inline comments to D53629: [GlobalISel] Restrict G_MERGE_VALUES capability and replace with new opcodes.
Nov 6 2018, 3:59 PM
volkan created D54174: [GlobalISel] LegalizationArtifactCombiner: Combine aext([asz]ext x) -> [asz]ext x.
Nov 6 2018, 11:55 AM
volkan committed rL346253: [AArch64][GlobalISel] Simplify and autogenerate the legalizer tests.
[AArch64][GlobalISel] Simplify and autogenerate the legalizer tests
Nov 6 2018, 11:01 AM
volkan committed rL346251: Reland r346166: [GlobalISel] Refactor the artifact combiner a bit by using….
Reland r346166: [GlobalISel] Refactor the artifact combiner a bit by using…
Nov 6 2018, 10:35 AM

Nov 5 2018

volkan committed rL346175: Revert "[GlobalISel] Refactor the artifact combiner a bit by using….
Revert "[GlobalISel] Refactor the artifact combiner a bit by using…
Nov 5 2018, 2:27 PM
volkan committed rL346166: [GlobalISel] Refactor the artifact combiner a bit by using MIPatternMatch.
[GlobalISel] Refactor the artifact combiner a bit by using MIPatternMatch
Nov 5 2018, 12:53 PM
volkan closed D54116: [GlobalISel] Refactor the artifact combiner a bit by using MIPatternMatch.
Nov 5 2018, 12:53 PM
volkan added a comment to D54116: [GlobalISel] Refactor the artifact combiner a bit by using MIPatternMatch.

Thanks. With this patch, are there any more users of getOpcodeDef? If not, we should probably remove that helper.

Nov 5 2018, 11:52 AM
volkan added inline comments to D54116: [GlobalISel] Refactor the artifact combiner a bit by using MIPatternMatch.
Nov 5 2018, 11:46 AM
volkan updated the diff for D54116: [GlobalISel] Refactor the artifact combiner a bit by using MIPatternMatch.

Added the missing legality check.

Nov 5 2018, 11:45 AM
volkan created D54116: [GlobalISel] Refactor the artifact combiner a bit by using MIPatternMatch.
Nov 5 2018, 11:36 AM

Nov 2 2018

volkan 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.

+1 for having explicit opcodes for truncation.

Nov 2 2018, 11:00 AM

Nov 1 2018

volkan added a reviewer for D53728: [GlobalISel] LegalizerHelper: Implement fewerElementsVector for G_LOAD/G_STORE: aditya_nandakumar.
Nov 1 2018, 1:59 PM
volkan updated the diff for D53728: [GlobalISel] LegalizerHelper: Implement fewerElementsVector for G_LOAD/G_STORE.

Rebased.

Nov 1 2018, 12:48 PM
volkan committed rL345875: [GlobalISel] Fix a bug in LegalizeRuleSet::clampMaxNumElements.
[GlobalISel] Fix a bug in LegalizeRuleSet::clampMaxNumElements
Nov 1 2018, 12:06 PM
volkan closed D53734: [GlobalISel] Fix a bug in LegalizeRuleSet::clampMaxNumElements.
Nov 1 2018, 12:05 PM
volkan added inline comments to D53734: [GlobalISel] Fix a bug in LegalizeRuleSet::clampMaxNumElements.
Nov 1 2018, 10:45 AM

Oct 31 2018

volkan committed rL345751: [InstCombine] Combine nested min/max intrinsics with constants.
[InstCombine] Combine nested min/max intrinsics with constants
Oct 31 2018, 10:53 AM
volkan closed D53774: [InstCombine] Combine nested min/max intrinsics with constants.
Oct 31 2018, 10:53 AM
volkan added a comment to D53774: [InstCombine] Combine nested min/max intrinsics with constants.

Is it worth adjusting/adding some tests with NaN constants?

Oct 31 2018, 10:03 AM

Oct 30 2018

volkan added inline comments to D53774: [InstCombine] Combine nested min/max intrinsics with constants.
Oct 30 2018, 11:27 AM
volkan added a comment to D53774: [InstCombine] Combine nested min/max intrinsics with constants.

Side note: do you know if the reassociate pass handles these? I imagine we're going to miss the optimization in instcombine if the constants are separated in a longer chain of min/max ops.

Oct 30 2018, 11:17 AM
volkan updated the diff for D53774: [InstCombine] Combine nested min/max intrinsics with constants.
  • Updated the tests.
  • Added missing copyIRFlags call.
Oct 30 2018, 11:16 AM
volkan committed rL345616: [InstCombine] Add preliminary tests for nested min/max combines. NFC.
[InstCombine] Add preliminary tests for nested min/max combines. NFC
Oct 30 2018, 10:53 AM
volkan closed D53875: [InstCombine] Add preliminary tests for nested min/max combines. NFC.
Oct 30 2018, 10:53 AM
volkan created D53875: [InstCombine] Add preliminary tests for nested min/max combines. NFC.
Oct 30 2018, 10:40 AM

Oct 26 2018

volkan created D53774: [InstCombine] Combine nested min/max intrinsics with constants.
Oct 26 2018, 12:07 PM

Oct 25 2018

volkan created D53734: [GlobalISel] Fix a bug in LegalizeRuleSet::clampMaxNumElements.
Oct 25 2018, 2:55 PM
volkan created D53728: [GlobalISel] LegalizerHelper: Implement fewerElementsVector for G_LOAD/G_STORE.
Oct 25 2018, 1:43 PM
volkan committed rL345307: [AArch64][GlobalISel] Simplify a legalizer test. NFC..
[AArch64][GlobalISel] Simplify a legalizer test. NFC.
Oct 25 2018, 1:04 PM
volkan committed rL345292: [GlobalISel] LegalizerHelper: Fix the incorrect alignment when splitting….
[GlobalISel] LegalizerHelper: Fix the incorrect alignment when splitting…
Oct 25 2018, 10:54 AM
volkan closed D53664: [GlobalISel] LegalizerHelper: Fix the incorrect alignment when splitting loads/stores in narrowScalar.
Oct 25 2018, 10:54 AM
volkan committed rL345288: [GISel] LegalizerInfo: Rename MemDesc::Size to SizeInBits to make the value….
[GISel] LegalizerInfo: Rename MemDesc::Size to SizeInBits to make the value…
Oct 25 2018, 10:39 AM
volkan committed rL345282: [AArch64][GlobalISel] Fix the LegalityPredicate for lowerIf for G_LOAD/G_STORE.
[AArch64][GlobalISel] Fix the LegalityPredicate for lowerIf for G_LOAD/G_STORE
Oct 25 2018, 10:25 AM
volkan closed D53679: [AArch64][GlobalISel] Fix the LegalityPredicate for lowerIf for G_LOAD/G_STORE.
Oct 25 2018, 10:25 AM
volkan added a comment to D53679: [AArch64][GlobalISel] Fix the LegalityPredicate for lowerIf for G_LOAD/G_STORE.

LGTM with the test nit

Would you be willing to rename that Size member to SizeInBits to make the value clearer?

Oct 25 2018, 10:21 AM
volkan updated the diff for D53679: [AArch64][GlobalISel] Fix the LegalityPredicate for lowerIf for G_LOAD/G_STORE.

Updated the test to include the full instruction.

Oct 25 2018, 10:21 AM

Oct 24 2018

volkan created D53679: [AArch64][GlobalISel] Fix the LegalityPredicate for lowerIf for G_LOAD/G_STORE.
Oct 24 2018, 3:41 PM
volkan updated the diff for D53664: [GlobalISel] LegalizerHelper: Fix the incorrect alignment when splitting loads/stores in narrowScalar.
  • Simplified the test.
  • Replaced GreatestCommonDivisor64 with MinAlign to make sure the new alignment is power of 2.
Oct 24 2018, 1:27 PM
volkan created D53664: [GlobalISel] LegalizerHelper: Fix the incorrect alignment when splitting loads/stores in narrowScalar.
Oct 24 2018, 12:07 PM
volkan accepted D53592: [GlobalISel] Use the target preferred type for G_EXTRACT_VECTOR_ELT index.

LGTM, thanks Amara.

Oct 24 2018, 11:01 AM