Page MenuHomePhabricator

bjope (Bjorn Pettersson)
User

Projects

User does not belong to any projects.

User Details

User Since
Sep 26 2016, 7:58 AM (237 w, 2 d)

Recent Activity

Mon, Apr 12

bjope added a comment to D99438: [SimplifyLibCalls] Take size of int into consideration when emitting ldexp/ldexpf.

I think you should ask on llvm-dev about this :)

Mon, Apr 12, 1:37 PM · Restricted Project
bjope added inline comments to D99438: [SimplifyLibCalls] Take size of int into consideration when emitting ldexp/ldexpf.
Mon, Apr 12, 1:52 AM · Restricted Project

Wed, Apr 7

bjope added a comment to D99674: [InstCombine] Conditionally fold select i1 into and/or.

Here is a reduced reproducer that goes into infinite loop with this patch:

Wed, Apr 7, 4:38 AM · Restricted Project
bjope added a comment to D99674: [InstCombine] Conditionally fold select i1 into and/or.

I've seen problems with infinite loops in InstCombine after mergin this patch downstream:

Wed, Apr 7, 12:36 AM · Restricted Project

Tue, Apr 6

bjope added a comment to D99438: [SimplifyLibCalls] Take size of int into consideration when emitting ldexp/ldexpf.

Seems like there is little interest in this, but I was not sure really who to call for review from the beginning. Please let me know if you aren't interested in reviewing this by removing yourselves as reviewer (that way I know I need to find some other candidates).

Tue, Apr 6, 2:40 AM · Restricted Project
bjope added inline comments to D98114: [Loads] Forward constant vector store to load of first element.
Tue, Apr 6, 12:41 AM · Restricted Project

Mon, Apr 5

bjope added a comment to D99886: [CMake] Propagate static extensions to LLVMExtensions object library.

@bjope Could you try without your workaround from https://reviews.llvm.org/rGd66f9c4f1e83e69abf75f97cb5f8fd1dc9422357 with this patch? I think this should fix the lack of include directory propagation that likely caused your issue.

Mon, Apr 5, 9:07 AM · Restricted Project

Sat, Apr 3

bjope added inline comments to D98114: [Loads] Forward constant vector store to load of first element.
Sat, Apr 3, 7:59 AM · Restricted Project
bjope added inline comments to D98114: [Loads] Forward constant vector store to load of first element.
Sat, Apr 3, 4:42 AM · Restricted Project
bjope added a comment to D95727: llvm-shlib: Create object libraries for each component and link against them.

I pushed my workaround for Z3 here: https://reviews.llvm.org/rGd66f9c4f1e83e69abf75f97cb5f8fd1dc9422357
Hopefully that will make our bots happy again.

Sat, Apr 3, 3:29 AM · Restricted Project
bjope committed rGd66f9c4f1e83: Fix build rules for LLVM_WITH_Z3 after D95727 (authored by bjope).
Fix build rules for LLVM_WITH_Z3 after D95727
Sat, Apr 3, 3:26 AM
bjope added a comment to D95727: llvm-shlib: Create object libraries for each component and link against them.

Not sure if it is the correct thing to do, but a patch like this seem to help for the Z3 problem:

Sat, Apr 3, 3:07 AM · Restricted Project
bjope added a comment to D95727: llvm-shlib: Create object libraries for each component and link against them.

So this seem to break more things than just Z3 builds (considering D99829). Should we revert while sorting out these things. I'm kind of lost here, just know that our integration bots are stuck and they've been failing since this patch landed on April 1.

Sat, Apr 3, 1:53 AM · Restricted Project

Fri, Apr 2

bjope updated subscribers of D95727: llvm-shlib: Create object libraries for each component and link against them.
Fri, Apr 2, 6:47 AM · Restricted Project
bjope added a comment to D95727: llvm-shlib: Create object libraries for each component and link against them.

Unfortunately out downstream bots started to fail with this commit (building the main branch).

Fri, Apr 2, 6:45 AM · Restricted Project

Wed, Mar 31

bjope abandoned D97690: [SLP] Add test case showing shortcoming in honoring max reg size.

This one actually landed as https://reviews.llvm.org/rG2f8f01dcb3d43d2fb1149fc8988e61f93f9064f5

Wed, Mar 31, 12:07 AM · Restricted Project

Mon, Mar 29

bjope added a comment to D99451: Use write_basic_package_version_file for LLVM.

Here are some diffs I get depending on having this patch or not:

Mon, Mar 29, 5:02 AM · Restricted Project
bjope added a comment to D99451: Use write_basic_package_version_file for LLVM.

We are also seeing problems with this when rebasing downstream. A somewhat strange thing was that it worked if I invoked the build a second time without removing my build directory in between. But with at clean (non existing) build dir I got failures.
I thought that it maybe was something wrong with my merge (or just a downstream problem), but if others also see problems with this then maybe not. Anyway, if I revert this patch things works fine again.

Just wanted to highlight that one might need to remove the build dir between tests when trying to reproduce the problem as my experience is that when invoking the build a second time it might pass. So some kind of dependency problem perhaps?

In our case we always do a clean build on bots so I'm not sure if it's the same issue.

Mon, Mar 29, 2:19 AM · Restricted Project
bjope added a comment to D99451: Use write_basic_package_version_file for LLVM.

We are also seeing problems with this when rebasing downstream. A somewhat strange thing was that it worked if I invoked the build a second time without removing my build directory in between. But with at clean (non existing) build dir I got failures.
I thought that it maybe was something wrong with my merge (or just a downstream problem), but if others also see problems with this then maybe not. Anyway, if I revert this patch things works fine again.

Mon, Mar 29, 1:26 AM · Restricted Project
bjope added reviewers for D99439: Update @llvm.powi to handle different int sizes for the exponent: atrosinenko, xbolva00, efriedma.
Mon, Mar 29, 1:13 AM · Restricted Project, Restricted Project
bjope added reviewers for D99438: [SimplifyLibCalls] Take size of int into consideration when emitting ldexp/ldexpf: evandro, xbolva00, efriedma, atrosinenko, dylanmckay.
Mon, Mar 29, 12:52 AM · Restricted Project

Fri, Mar 26

bjope requested review of D99439: Update @llvm.powi to handle different int sizes for the exponent.
Fri, Mar 26, 1:09 PM · Restricted Project, Restricted Project
bjope requested review of D99438: [SimplifyLibCalls] Take size of int into consideration when emitting ldexp/ldexpf.
Fri, Mar 26, 1:08 PM · Restricted Project

Wed, Mar 24

bjope added inline comments to D98367: [libcxxabi] Use cxx-headers target to consume libcxx headers.
Wed, Mar 24, 4:05 AM · Restricted Project

Tue, Mar 23

bjope accepted D99163: [LangRef] Fix typos in the vector-type memory layout section.

LGTM! (Thanks!)

Tue, Mar 23, 3:12 AM · Restricted Project

Mon, Mar 22

bjope committed rG688cdddafb0d: [SLP] Honor min/max regsize and min/max VF in vectorizeStores (authored by bjope).
[SLP] Honor min/max regsize and min/max VF in vectorizeStores
Mon, Mar 22, 9:30 AM
bjope committed rG2f8f01dcb3d4: [SLP] Add test case showing shortcoming in honoring max reg size (authored by bjope).
[SLP] Add test case showing shortcoming in honoring max reg size
Mon, Mar 22, 9:30 AM
bjope closed D97691: [SLP] Honor min/max regsize and min/max VF in vectorizeStores.
Mon, Mar 22, 9:30 AM · Restricted Project
bjope added a comment to D81285: [builtins] Change si_int to int in some helper declarations.

My users have found problems with miscompiles that seem to originate from this patch.

Mon, Mar 22, 9:15 AM · Restricted Project

Fri, Mar 19

bjope committed rG5737010a7948: [LangRef] Describe memory layout for vectors types (authored by bjope).
[LangRef] Describe memory layout for vectors types
Fri, Mar 19, 11:01 AM
bjope closed D94964: [LangRef] Describe memory layout for vectors types.
Fri, Mar 19, 11:01 AM · Restricted Project
bjope updated the diff for D94964: [LangRef] Describe memory layout for vectors types.

Minor update (using a bit more graphical examples instead of only text, even though it isn't exactly ascii art as someone suggested).

Fri, Mar 19, 10:54 AM · Restricted Project

Wed, Mar 17

bjope added a comment to D94964: [LangRef] Describe memory layout for vectors types.

Overall LGTM. Thanks for documenting this! It was painful to reverse-engineer this when implementing it in Alive2..

My only concern is about leaving vector sizes not multiple of a byte as unspecified. Doing so is the same as saying those are illegal; can't be used in practice. Can we define those as being padded, where the padding is poison, like in structs? That allows using vectors where padding isn't easy, like <8 x i3>. How do you store that to memory if the direct store is left unspecified? And with people using weird integer sizes for ML & FPGAs, I think it's a good idea to define this case.
Alive2 is being conservative here and defining the padding as zero, but it should be poison IMHO.

Wed, Mar 17, 1:53 PM · Restricted Project
bjope added a comment to D97891: Add register size info back to MCRegisterClass.

I'm inclined to accept this as is. There is definitely a limitation here that cannot be easily worked around.

Wed, Mar 17, 11:39 AM · Restricted Project
bjope added inline comments to D94964: [LangRef] Describe memory layout for vectors types.
Wed, Mar 17, 9:53 AM · Restricted Project
bjope added inline comments to D94964: [LangRef] Describe memory layout for vectors types.
Wed, Mar 17, 2:19 AM · Restricted Project

Tue, Mar 16

bjope retitled D94964: [LangRef] Describe memory layout for vectors types from Describe vector layout in LangRef to [LangRef] Describe memory layout for vectors types.
Tue, Mar 16, 7:37 AM · Restricted Project
bjope updated the diff for D94964: [LangRef] Describe memory layout for vectors types.

Changed the wording quite a bit. Getting rid of references to C ABI etc.

Tue, Mar 16, 7:36 AM · Restricted Project
bjope commandeered D94964: [LangRef] Describe memory layout for vectors types.

I'll try to make some progress here.

Tue, Mar 16, 7:33 AM · Restricted Project
bjope committed rG5ac3b37599d3: [TableGen/GlobalISel] Emit MI_predicate custom code for PatFrags (not only… (authored by bjope).
[TableGen/GlobalISel] Emit MI_predicate custom code for PatFrags (not only…
Tue, Mar 16, 4:45 AM
bjope closed D98486: [TableGen/GlobalISel] Emit MI_predicate custom code for PatFrags (not only PatFrag).
Tue, Mar 16, 4:44 AM · Restricted Project

Mar 14 2021

bjope added inline comments to D98423: Fix the trunc instruction insertion problem in SLP pass.
Mar 14 2021, 5:28 AM · Restricted Project

Mar 12 2021

bjope added inline comments to D98486: [TableGen/GlobalISel] Emit MI_predicate custom code for PatFrags (not only PatFrag).
Mar 12 2021, 6:00 AM · Restricted Project
bjope requested review of D98486: [TableGen/GlobalISel] Emit MI_predicate custom code for PatFrags (not only PatFrag).
Mar 12 2021, 3:29 AM · Restricted Project
bjope committed rG529c8e8dc6e9: [InstSimplify] Simplify smul.fix and smul.fix.sat (authored by bjope).
[InstSimplify] Simplify smul.fix and smul.fix.sat
Mar 12 2021, 12:11 AM
bjope committed rG3638bdfbda01: [ConstantFold] Handle undef/poison when constant folding smul_fix/smul_fix_sat (authored by bjope).
[ConstantFold] Handle undef/poison when constant folding smul_fix/smul_fix_sat
Mar 12 2021, 12:11 AM
bjope closed D98299: [InstSimplify] Simplify smul.fix and smul.fix.sat.
Mar 12 2021, 12:11 AM · Restricted Project
bjope closed D98410: [ConstantFold] Handle undef/poison when constant folding smul_fix/smul_fix_sat.
Mar 12 2021, 12:11 AM · Restricted Project
bjope added inline comments to D98410: [ConstantFold] Handle undef/poison when constant folding smul_fix/smul_fix_sat.
Mar 12 2021, 12:09 AM · Restricted Project

Mar 11 2021

bjope added inline comments to D98410: [ConstantFold] Handle undef/poison when constant folding smul_fix/smul_fix_sat.
Mar 11 2021, 8:17 AM · Restricted Project
bjope added a comment to D98410: [ConstantFold] Handle undef/poison when constant folding smul_fix/smul_fix_sat.

So undef include values that isn't possible for every combination of the other operands.

Aha, I see! This is an interesting reasoning. I think it is fine to do what you're doing for the sake of simplicity, what follows under the horizontal line below is a bunch of my rambling (feel free to ignore all of it)


Taking a regular mul instruction as an example

mul i32 1, undef  # => undef
mul i32 2, undef  # => 0
mul i32 3, undef  # => undef

These make sense to me, though require some roundabout reasoning. 1 * undef = undef is trivial. 2 * undef follows the same reasoning as yours in that there's bit-pattern undef can take that would result in lower bit set here. That same reasoning could apply for 3 * undef too – but it does not because of wrap-around (I think).

And this is where the following snippet from the smul.fix definition comes in:

It is undefined behavior if the result value does not fit within the range of the fixed point type.

Basically, unless the intrinsic is multiplying undef by 1 or by 0, I believe there's an implicit invocation of UB. That's because there exist bit patterns of an undef operand that multiplied by other values would cause the resulting value to not fit within the fixed point type's range (right?). Whether we manifest this UB as undef or 0 probably doesn't matter too much, but I think either option would be valid.

Mar 11 2021, 7:00 AM · Restricted Project
bjope added a comment to D98410: [ConstantFold] Handle undef/poison when constant folding smul_fix/smul_fix_sat.

Wouldn't folding C * undef and undef * C into an undef, rather than 0, be valid/better? Is there a reason we cannot do that?

Mar 11 2021, 4:44 AM · Restricted Project
bjope added inline comments to D98410: [ConstantFold] Handle undef/poison when constant folding smul_fix/smul_fix_sat.
Mar 11 2021, 4:04 AM · Restricted Project
bjope updated the diff for D98299: [InstSimplify] Simplify smul.fix and smul.fix.sat.

Update code comments (those comments depends on landing D98410 first).

Mar 11 2021, 4:01 AM · Restricted Project
bjope requested review of D98410: [ConstantFold] Handle undef/poison when constant folding smul_fix/smul_fix_sat.
Mar 11 2021, 3:58 AM · Restricted Project
bjope added inline comments to D98299: [InstSimplify] Simplify smul.fix and smul.fix.sat.
Mar 11 2021, 1:44 AM · Restricted Project

Mar 9 2021

bjope requested review of D98299: [InstSimplify] Simplify smul.fix and smul.fix.sat.
Mar 9 2021, 3:15 PM · Restricted Project

Mar 7 2021

bjope added a comment to D95789: [SpeculateAroundPHIs] Avoid speculation on loop back edges.

IIUC, this patch inhibits optimizations opportunities because of a bug elsewhere?

Mar 7 2021, 8:51 AM · Restricted Project

Mar 4 2021

bjope added a comment to D95789: [SpeculateAroundPHIs] Avoid speculation on loop back edges.

Some passes passes might not handle critical edges, in addition to SpeculateAroundPHIs . Wouldn't it be simpler to fix llvm::SplitCriticalEdge to make it put the loop metadata to the correct branch?

Mar 4 2021, 12:39 PM · Restricted Project
bjope added a comment to D97722: [NewPM] Revamp pass names.

I guess it wouldn't hurt if the pass names generally would match with the DEBUG_TYPE.

Mar 4 2021, 12:22 PM · Restricted Project
bjope added a comment to D97891: Add register size info back to MCRegisterClass.

If you need this I'd rather "add the needed support" than doing it as a revert (although nice to get the history here in the review). To be more specific; I would let RegisterInfoEmitter provide the size in bits rather than the mystierious "/8" that for example results in a 1-bit register being reported as having size 0. And I'd get rid of the redundant "getSize()" method. And the change to the code comment in TargetInstrInfo::getStackSlotRange could probably be skipped.

Mar 4 2021, 12:22 AM · Restricted Project

Mar 1 2021

bjope requested review of D97691: [SLP] Honor min/max regsize and min/max VF in vectorizeStores.
Mar 1 2021, 7:49 AM · Restricted Project
bjope requested review of D97690: [SLP] Add test case showing shortcoming in honoring max reg size.
Mar 1 2021, 7:45 AM · Restricted Project

Feb 19 2021

bjope added a reviewer for D95789: [SpeculateAroundPHIs] Avoid speculation on loop back edges: Meinersbur.
Feb 19 2021, 7:24 AM · Restricted Project

Feb 10 2021

bjope added inline comments to D96101: [opt][NewPM] Add a --print-passes flag to print all available passes.
Feb 10 2021, 11:58 AM · Restricted Project
bjope accepted D95217: [LoopVectorize] Fix VPRecipeBuilder::createEdgeMask to correctly generate the mask.

LGTM!

Feb 10 2021, 12:31 AM · Restricted Project

Feb 8 2021

bjope added a comment to D94765: Expand masked mem intrinsics correctly wrt big-endian.

OK. Thanks for the commit. This LGTM.

Are you happy for me to commit this and D94867 together, to keep them in-sync?

Feb 8 2021, 11:57 AM · Restricted Project

Feb 7 2021

bjope added inline comments to D96100: Specify that some flags are legacy PM-specific.
Feb 7 2021, 3:33 PM · Restricted Project
bjope added inline comments to D96101: [opt][NewPM] Add a --print-passes flag to print all available passes.
Feb 7 2021, 3:06 PM · Restricted Project

Feb 5 2021

bjope updated subscribers of D96101: [opt][NewPM] Add a --print-passes flag to print all available passes.
Feb 5 2021, 12:44 AM · Restricted Project
bjope added reviewers for D95789: [SpeculateAroundPHIs] Avoid speculation on loop back edges: fhahn, reames, spatel, chandlerc, asbirlea.

I think the problem with loop metadata not being handled properly is a bug (that also could impact performance negatively) , so this is a quick fix (but I guess it could be possible to handle transition of metadata to the new latch with some effort if the transform is beneficial).

Feb 5 2021, 12:36 AM · Restricted Project

Feb 1 2021

bjope added a comment to rG1487747e990c: [LTO] Prevent devirtualization for symbols dynamically exported.

The fix for both --export-dynamic-symbol and --dynamic-list went in
sometime 2017, so it would not be in v1.12 but should be by v1.16. I will
split this part of the test out into a separate test that I move into the
v1.16 subdirectory.

Feb 1 2021, 9:50 AM
bjope added a comment to rG1487747e990c: [LTO] Prevent devirtualization for symbols dynamically exported.

Oh, I suspect this might be due to an older version of gold. ISTR now that
--export-dynamic-symbol behavior when using a plugin was fixed at some
point - this part of the test might need to be extracted and put into
either the v1.12 or v1.16 subdirectory. Can you check your version of gold
and let me know what it says? I'll see if I can figure out if and when that
was fixed in gold meanwhile.

Feb 1 2021, 9:42 AM
bjope added inline comments to rG1487747e990c: [LTO] Prevent devirtualization for symbols dynamically exported.
Feb 1 2021, 9:25 AM
bjope requested review of D95789: [SpeculateAroundPHIs] Avoid speculation on loop back edges.
Feb 1 2021, 7:58 AM · Restricted Project

Jan 31 2021

bjope updated the diff for D94765: Expand masked mem intrinsics correctly wrt big-endian.

Add tests using big endian datalayout.

Jan 31 2021, 3:12 PM · Restricted Project

Jan 28 2021

bjope added inline comments to D95117: [NewPM][opt] Run the "default" AA pipeline by default.
Jan 28 2021, 1:09 PM · Restricted Project
bjope added inline comments to D95117: [NewPM][opt] Run the "default" AA pipeline by default.
Jan 28 2021, 4:36 AM · Restricted Project

Jan 27 2021

bjope added a comment to D95217: [LoopVectorize] Fix VPRecipeBuilder::createEdgeMask to correctly generate the mask.

Btw, as far as I know @uabelho hasn't found any problems with the fix. And except the checks in the test case I think it looks like a nice fix (avoiding miscompiles).

Jan 27 2021, 8:56 AM · Restricted Project
bjope added inline comments to D95217: [LoopVectorize] Fix VPRecipeBuilder::createEdgeMask to correctly generate the mask.
Jan 27 2021, 8:54 AM · Restricted Project

Jan 26 2021

bjope committed rGa9bd3d37bdec: [NewPM] Add ExtraVectorizerPasses support (authored by bjope).
[NewPM] Add ExtraVectorizerPasses support
Jan 26 2021, 1:59 PM
bjope closed D95457: [NewPM] Add ExtraVectorizerPasses support.
Jan 26 2021, 1:59 PM · Restricted Project
bjope added inline comments to D95457: [NewPM] Add ExtraVectorizerPasses support.
Jan 26 2021, 10:38 AM · Restricted Project
bjope updated the diff for D95457: [NewPM] Add ExtraVectorizerPasses support.

Update correct version of test case (without the FIXME).

Jan 26 2021, 10:36 AM · Restricted Project
bjope requested review of D95457: [NewPM] Add ExtraVectorizerPasses support.
Jan 26 2021, 10:04 AM · Restricted Project
bjope added inline comments to rG8871a4b4cab8: [Constant] Update ConstantVector::get to return poison if all input elems are….
Jan 26 2021, 7:19 AM
bjope added inline comments to rG8871a4b4cab8: [Constant] Update ConstantVector::get to return poison if all input elems are….
Jan 26 2021, 4:30 AM

Jan 22 2021

bjope committed rGea2cfda386f1: [CGExpr] Use getCharWidth() more consistently in CCGExprConstant. NFC (authored by bjope).
[CGExpr] Use getCharWidth() more consistently in CCGExprConstant. NFC
Jan 22 2021, 12:15 PM
bjope committed rG72f863fd37c3: [CodeGen] Use getCharWidth() more consistently in CGRecordLowering. NFC (authored by bjope).
[CodeGen] Use getCharWidth() more consistently in CGRecordLowering. NFC
Jan 22 2021, 12:15 PM
bjope closed D94979: [CGExpr] Use getCharWidth() more consistently in CCGExprConstant. NFC.
Jan 22 2021, 12:15 PM · Restricted Project
bjope closed D94977: [CodeGen] Use getCharWidth() more consistently in CGRecordLowering. NFC.
Jan 22 2021, 12:14 PM · Restricted Project

Jan 21 2021

bjope added a comment to D94979: [CGExpr] Use getCharWidth() more consistently in CCGExprConstant. NFC.

@lebedev.ri : I think I need your blessing as well on this, considering your earlier concerns. Is it still just confusing? (I doubt that we want/can replace all uses of getCharWidth/getIntWidth etc in clang with integers)

Jan 21 2021, 9:43 AM · Restricted Project
bjope added a comment to D92309: [LegacyPM] Update InversedLastUser on the fly. NFC..

Still LGTM

Jan 21 2021, 9:33 AM · Restricted Project
bjope retitled D94979: [CGExpr] Use getCharWidth() more consistently in CCGExprConstant. NFC from [CGExpr] Use getCharWidth() more consistently in ConstantAggregateBuilderUtils. NFC to [CGExpr] Use getCharWidth() more consistently in CCGExprConstant. NFC.
Jan 21 2021, 2:01 AM · Restricted Project
bjope updated the diff for D94979: [CGExpr] Use getCharWidth() more consistently in CCGExprConstant. NFC.

Updated to add CharTy to the CodeGenTypeCache (based on review feedback).

Jan 21 2021, 2:00 AM · Restricted Project

Jan 20 2021

bjope added a comment to D92309: [LegacyPM] Update InversedLastUser on the fly. NFC..

LGTM

Thanks. This patch depends on D92308. Without D92308, this patch would add lots of extra lines to the output of opt -debug-pass=Structure, which breaks various tests in test/CodeGen and test/Other.

Jan 20 2021, 6:49 AM · Restricted Project
bjope accepted D92309: [LegacyPM] Update InversedLastUser on the fly. NFC..
Jan 20 2021, 4:59 AM · Restricted Project
bjope added a comment to D92309: [LegacyPM] Update InversedLastUser on the fly. NFC..

LGTM

Jan 20 2021, 4:58 AM · Restricted Project
bjope committed rG985b9b7e421a: [PM] Avoid duplicates in the Used/Preserved/Required sets (authored by bjope).
[PM] Avoid duplicates in the Used/Preserved/Required sets
Jan 20 2021, 4:56 AM
bjope closed D94416: [PM] Avoid duplicates in the Used/Preserved/Required sets.
Jan 20 2021, 4:56 AM · Restricted Project