Page MenuHomePhabricator

bjope (Bjorn Pettersson)
User

Projects

User does not belong to any projects.

User Details

User Since
Sep 26 2016, 7:58 AM (125 w, 5 d)

Recent Activity

Yesterday

bjope added inline comments to D55720: [Intrinsic] Signed Fixed Point Saturation Multiplication Intrinsic.
Fri, Feb 22, 4:28 PM · Restricted Project
bjope added a comment to D58453: [DebugInfo][CGP] Limit placeDbgValues movement of dbg.value intrinsics.

placeDbgValues has been bugging me (and probably others) for some time. So I'm happy with this patch.

Fri, Feb 22, 5:15 AM · Restricted Project
bjope added a comment to D58238: [DebugInfo] MachineSink: Insert undef DBG_VALUEs when sinking instructions, try to forward copies.

Maybe it is a philosophical question. Is the compiler scheduling/reordering source instructions (in the debugger it will appear as source statements are executed in a random order), or are we scheduling/reordering machine instructions (and in the debugger it still should appear as if we execute variable assignments in the order of the source code). VLIW-scheduling, tail merging, etc, of course make things extra complicated, since we basically will be all over the place all the time.

To me it's the latter, my mental model (maybe wrong) is that while the compiler is optimising code, for debuginfo it has a state-machine of variable locations represented by dbg.values that it needs to try and preserve the order of, marking locations undef if they don't exist any more. The logical consequence is that if we had a function where the compiler managed to completely reverse the order of computation, no variable would have a location, which would be sound, but not useful.

Fri, Feb 22, 1:56 AM · Restricted Project

Thu, Feb 21

bjope added inline comments to D58453: [DebugInfo][CGP] Limit placeDbgValues movement of dbg.value intrinsics.
Thu, Feb 21, 3:31 PM · Restricted Project

Wed, Feb 20

bjope added inline comments to D58403: [DebugInfo][CGP] Update dbg.values when updating memory address computations.
Wed, Feb 20, 12:17 AM · Restricted Project

Fri, Feb 15

bjope added a comment to D58238: [DebugInfo] MachineSink: Insert undef DBG_VALUEs when sinking instructions, try to forward copies.

I have some doubt about this. Mainly the part about inserting undefs. Although, some of my doubts are partially based on the big-black-hole regarding how debug info is supposed to work for heavily optimized code (and more specifically how this is supposed to work in LLVM based on what we got today).

Fri, Feb 15, 2:54 AM · Restricted Project

Thu, Feb 14

bjope added inline comments to D56788: [DebugInfo][InstCombine] Prefer salvaging dbg.values over sinking them.
Thu, Feb 14, 3:51 AM · Restricted Project
bjope added inline comments to D56788: [DebugInfo][InstCombine] Prefer salvaging dbg.values over sinking them.
Thu, Feb 14, 1:31 AM · Restricted Project

Wed, Feb 13

bjope accepted D57584: [DebugInfo][DAG] Reduce SelectionDAGs reordering of variables referring to Arguments.

LGTM! But remember to update the "summary" in the commit msg before commit. It is a little bit out-dated at the moment (e.g. the info about sparc test fixup).

Wed, Feb 13, 4:18 AM · Restricted Project
bjope added a comment to D57584: [DebugInfo][DAG] Reduce SelectionDAGs reordering of variables referring to Arguments.

The filtering from r353735 happily makes the sparc test change un-necessary.

I also tested whether filtering parameters so that they can't take VReg SDDbgValues is necessary any more. Removing the if block around the relevant code still makes DebugInfo/X86/sdag-split-arg.ll change behaviour. This doesn't surprise me: how fast-isel interacts with SelectionDAG is a mystery to me.

Wed, Feb 13, 4:12 AM · Restricted Project
Herald updated subscribers of D58041: [Backend] DBG_CALLSITE & DBG_CALLSITEPARAM instr handling.
Wed, Feb 13, 2:41 AM · debug-info

Tue, Feb 12

bjope added inline comments to D57584: [DebugInfo][DAG] Reduce SelectionDAGs reordering of variables referring to Arguments.
Tue, Feb 12, 2:45 PM · Restricted Project
bjope added a comment to D56788: [DebugInfo][InstCombine] Prefer salvaging dbg.values over sinking them.

LGTM as well!

Tue, Feb 12, 2:34 PM · Restricted Project
bjope committed rGecd0960718ba: [SelectionDAG] Clean up comments in SelectionDAGBuilder.h. NFC (authored by bjope).
[SelectionDAG] Clean up comments in SelectionDAGBuilder.h. NFC
Tue, Feb 12, 2:12 PM
bjope committed rL353886: [SelectionDAG] Clean up comments in SelectionDAGBuilder.h. NFC.
[SelectionDAG] Clean up comments in SelectionDAGBuilder.h. NFC
Tue, Feb 12, 2:12 PM

Mon, Feb 11

bjope committed rG4892f06e06cc: [SelectionDAGBuilder] Add restrictions to EmitFuncArgumentDbgValue (authored by bjope).
[SelectionDAGBuilder] Add restrictions to EmitFuncArgumentDbgValue
Mon, Feb 11, 11:23 AM
bjope committed rL353735: [SelectionDAGBuilder] Add restrictions to EmitFuncArgumentDbgValue.
[SelectionDAGBuilder] Add restrictions to EmitFuncArgumentDbgValue
Mon, Feb 11, 11:23 AM
bjope closed D57702: [SelectionDAGBuilder] Add restrictions to EmitFuncArgumentDbgValue.
Mon, Feb 11, 11:23 AM · Restricted Project
bjope added inline comments to D57702: [SelectionDAGBuilder] Add restrictions to EmitFuncArgumentDbgValue.
Mon, Feb 11, 10:29 AM · Restricted Project

Sat, Feb 9

bjope updated the diff for D57702: [SelectionDAGBuilder] Add restrictions to EmitFuncArgumentDbgValue.

Minor adjustments to the test case (added some missing CHECK-NOT).

Sat, Feb 9, 5:32 PM · Restricted Project
bjope added inline comments to D57702: [SelectionDAGBuilder] Add restrictions to EmitFuncArgumentDbgValue.
Sat, Feb 9, 5:13 PM · Restricted Project
bjope updated the diff for D57702: [SelectionDAGBuilder] Add restrictions to EmitFuncArgumentDbgValue.

Updated code comments.

Sat, Feb 9, 4:57 PM · Restricted Project
bjope added inline comments to D57702: [SelectionDAGBuilder] Add restrictions to EmitFuncArgumentDbgValue.
Sat, Feb 9, 4:25 PM · Restricted Project

Thu, Feb 7

bjope added inline comments to D56587: Introduce DW_OP_LLVM_convert.
Thu, Feb 7, 2:52 PM · debug-info
bjope added inline comments to D57702: [SelectionDAGBuilder] Add restrictions to EmitFuncArgumentDbgValue.
Thu, Feb 7, 5:22 AM · Restricted Project
bjope added inline comments to D57702: [SelectionDAGBuilder] Add restrictions to EmitFuncArgumentDbgValue.
Thu, Feb 7, 5:19 AM · Restricted Project
bjope added inline comments to D55720: [Intrinsic] Signed Fixed Point Saturation Multiplication Intrinsic.
Thu, Feb 7, 1:14 AM · Restricted Project
bjope added inline comments to D55720: [Intrinsic] Signed Fixed Point Saturation Multiplication Intrinsic.
Thu, Feb 7, 12:40 AM · Restricted Project
bjope added inline comments to D55720: [Intrinsic] Signed Fixed Point Saturation Multiplication Intrinsic.
Thu, Feb 7, 12:02 AM · Restricted Project

Wed, Feb 6

bjope committed rG350352c8a57c: [SelectionDAG] Cleanup some code comments. NFC (authored by bjope).
[SelectionDAG] Cleanup some code comments. NFC
Wed, Feb 6, 9:37 AM
bjope committed rL353317: [SelectionDAG] Cleanup some code comments. NFC.
[SelectionDAG] Cleanup some code comments. NFC
Wed, Feb 6, 9:36 AM
bjope updated the diff for D57702: [SelectionDAGBuilder] Add restrictions to EmitFuncArgumentDbgValue.

Revert the no longer needed modification in test/DebugInfo/Sparc/subreg.ll

Wed, Feb 6, 9:03 AM · Restricted Project
bjope updated the summary of D57702: [SelectionDAGBuilder] Add restrictions to EmitFuncArgumentDbgValue.
Wed, Feb 6, 9:03 AM · Restricted Project
bjope added inline comments to D57702: [SelectionDAGBuilder] Add restrictions to EmitFuncArgumentDbgValue.
Wed, Feb 6, 8:08 AM · Restricted Project
bjope updated the diff for D57702: [SelectionDAGBuilder] Add restrictions to EmitFuncArgumentDbgValue.

Added some code to make test/DebugInfo/AArch64/inlined-argument.ll pass.
I now allow emitting using ArgDbgValue if the dbg.value intrinsic is
in the beginning of the function entry (before any non-dbg instruction).
This is a little bit hacky, but the best solution I could find.

Wed, Feb 6, 7:55 AM · Restricted Project

Tue, Feb 5

bjope added inline comments to D56587: Introduce DW_OP_LLVM_convert.
Tue, Feb 5, 8:27 AM · debug-info

Mon, Feb 4

bjope added inline comments to D57584: [DebugInfo][DAG] Reduce SelectionDAGs reordering of variables referring to Arguments.
Mon, Feb 4, 12:03 PM · Restricted Project
bjope planned changes to D57702: [SelectionDAGBuilder] Add restrictions to EmitFuncArgumentDbgValue.

There is some problem with this making the test/DebugInfo/AArch64/inlined-argument.ll fail. The inlined "resources" parameter will appear optimized out. I haven't figured out why yet.

Mon, Feb 4, 11:46 AM · Restricted Project
bjope created D57702: [SelectionDAGBuilder] Add restrictions to EmitFuncArgumentDbgValue.
Mon, Feb 4, 11:42 AM · Restricted Project
bjope added a comment to D57584: [DebugInfo][DAG] Reduce SelectionDAGs reordering of variables referring to Arguments.

Maybe I was a little bit too quick to say that this solved our problem. Our problem is related to SelectionDAGBuilder::EmitFuncArgumentDbgValue hoiting dbg.value instructions referring to arguments (even if we just use the value from an argument to describe another variable that is assigned later on in the function). We will file a bugzilla ticket for that.

It may be that replicating the IsParamOfFunc logic in this change into resolveDanglingDebugInfo resolves that. (I haven't explored doing that yet).

Mon, Feb 4, 4:39 AM · Restricted Project

Fri, Feb 1

bjope added inline comments to D57584: [DebugInfo][DAG] Reduce SelectionDAGs reordering of variables referring to Arguments.
Fri, Feb 1, 10:03 AM · Restricted Project
bjope added a comment to D57584: [DebugInfo][DAG] Reduce SelectionDAGs reordering of variables referring to Arguments.

Nice! Me and @dstenb actually discussed this kind of problem earlier today. So quite fun (and surprising) to see a solution pop up in the inbox.

Fri, Feb 1, 8:10 AM · Restricted Project
bjope updated subscribers of D57584: [DebugInfo][DAG] Reduce SelectionDAGs reordering of variables referring to Arguments.

Nice! Me and @dstenb actually discussed this kind of problem earlier today. So quite fun (and surprising) to see a solution pop up in the inbox.

Fri, Feb 1, 7:29 AM · Restricted Project

Thu, Jan 31

bjope added inline comments to D57510: [DebugInfo] Keep parameter DBG_VALUEs before prologue code.
Thu, Jan 31, 10:31 AM · Restricted Project, debug-info
bjope added inline comments to D56788: [DebugInfo][InstCombine] Prefer salvaging dbg.values over sinking them.
Thu, Jan 31, 12:18 AM · Restricted Project

Wed, Jan 30

bjope added inline comments to D57219: [Fixed Point Arithmetic] Fixed Point Comparisons.
Wed, Jan 30, 12:16 AM · Restricted Project

Tue, Jan 29

bjope committed rL352469: [IPCP] Don't crash due to arg count/type mismatch between caller/callee.
[IPCP] Don't crash due to arg count/type mismatch between caller/callee
Tue, Jan 29, 2:19 AM
bjope closed D57052: [IPCP] Don't crash due to arg count/type mismatch between caller/callee.
Tue, Jan 29, 2:19 AM

Mon, Jan 28

bjope added a comment to D57052: [IPCP] Don't crash due to arg count/type mismatch between caller/callee.

Missing testcase for varargs.

Mon, Jan 28, 10:34 AM
bjope updated the diff for D57052: [IPCP] Don't crash due to arg count/type mismatch between caller/callee.

Added test cases to verify that we still optmize for VLA (as long as all non-variadic arguments are provided).

Mon, Jan 28, 10:21 AM
bjope requested review of D57052: [IPCP] Don't crash due to arg count/type mismatch between caller/callee.

Please have another look.

Mon, Jan 28, 7:50 AM
bjope retitled D57052: [IPCP] Don't crash due to arg count/type mismatch between caller/callee from [IPCP] Don't crash due to arg count mismath between caller/callee to [IPCP] Don't crash due to arg count/type mismatch between caller/callee.
Mon, Jan 28, 7:46 AM
bjope updated the diff for D57052: [IPCP] Don't crash due to arg count/type mismatch between caller/callee.

Updates:

  • Fix for VarArgs (need to allow more actual than formal arguments).
  • Added check for matching argument types (including a new test case).
Mon, Jan 28, 7:43 AM

Jan 23 2019

bjope added a comment to D57052: [IPCP] Don't crash due to arg count/type mismatch between caller/callee.

I see your point and I acknowledge it is a problem. However, I'm not sure a check in IPCP and consequently later in other IPOs is the best solution.
While I have some ideas on how to deal with this (one is to not create call sites for this situation), I think we can go ahead with your local fix now,
keep the test case and make it more general as needed.

Long story short, LGTM.

One thing, can you mention that this problem was exposed by the fact that AbstractCallSites will look through the bitcast at the callee position of a call/invoke somewhere?

Jan 23 2019, 2:22 AM
bjope updated the summary of D57052: [IPCP] Don't crash due to arg count/type mismatch between caller/callee.
Jan 23 2019, 2:09 AM

Jan 22 2019

bjope created D57052: [IPCP] Don't crash due to arg count/type mismatch between caller/callee.
Jan 22 2019, 6:22 AM
bjope added a comment to D52109: [TwoAddressInstructionPass] Don't update SrcRegMap for copies inserted for tied register constraint when the src isn't killed.

Have you considered making a .mir-test that show the effect of TwoAddressInstruction pass here. Maybe an overkill since there already are plenty of test cases that are affected by the change, but it would make it easier to understand the tranformation difference that we introduce in the TwoAddressInstructionPass.

Jan 22 2019, 2:03 AM

Jan 21 2019

bjope added a comment to D55720: [Intrinsic] Signed Fixed Point Saturation Multiplication Intrinsic.

Should https://reviews.llvm.org/D56987 be a parent for this? Then you'd need to rebase getExpandedFixedPointMultiplication since that has changed into converting into MUL when scale is zero (that is not valid for saturation).

Jan 21 2019, 12:54 AM · Restricted Project
bjope added a comment to D56987: [Intrinsic] Expand SMULFIX to MUL, MULH[US], or [US]MUL_LOHI on vector arguments.

How common would it be that the scale is zero? Is that really expected in reality or just in this kind of handwritten test cases?

Jan 21 2019, 12:47 AM

Jan 18 2019

bjope committed rL351581: [SelectionDAG] Updates for -dag-dump-verbose.
[SelectionDAG] Updates for -dag-dump-verbose
Jan 18 2019, 12:10 PM
bjope closed D56793: [SelectionDAG] Updates for -dag-dump-verbose.
Jan 18 2019, 12:10 PM
bjope retitled D56793: [SelectionDAG] Updates for -dag-dump-verbose from [SelectionDAG] Add option -dag-dump-verbose-dbg to [SelectionDAG] Updates for -dag-dump-verbose.
Jan 18 2019, 6:33 AM
bjope updated the diff for D56793: [SelectionDAG] Updates for -dag-dump-verbose.

Some more updates (some tweaks after having looked at some logs).

Jan 18 2019, 6:32 AM

Jan 17 2019

bjope updated the diff for D56793: [SelectionDAG] Updates for -dag-dump-verbose.

Now refactored based on trunk (the first version I uploaded was a
quite old patch that didn't even compile, sorry for that).

Jan 17 2019, 3:53 PM
bjope added inline comments to D56793: [SelectionDAG] Updates for -dag-dump-verbose.
Jan 17 2019, 9:04 AM

Jan 16 2019

bjope added a comment to D56793: [SelectionDAG] Updates for -dag-dump-verbose.

@jmorse : Since you are working a little bit with the SelectionDAGBuilder and debug values etc, maybe you will find this patch helpful. It gives some more printouts for the "missing" DBG_VALUE nodes that normally isn't shown during ISel simply by using -debug or -dag-dump-verbose.

Jan 16 2019, 9:41 AM
bjope created D56793: [SelectionDAG] Updates for -dag-dump-verbose.
Jan 16 2019, 9:31 AM
bjope accepted D56678: [DebugInfo][DAG] Allow creation of DBG_VALUEs in blocks where the operand is not used.

PS. I found my old git branch where I played around with this, and I think that I never figured out that I needed the !isa<Argument> check. Maybe that is why I had mixed feeling about the result back in the days.

Jan 16 2019, 8:36 AM

Jan 15 2019

bjope added inline comments to D56587: Introduce DW_OP_LLVM_convert.
Jan 15 2019, 2:22 PM · debug-info

Jan 14 2019

bjope added inline comments to D56151: [DebugInfo] PR40010: Avoid register coalesing altering DBG_VALUE valuations.
Jan 14 2019, 12:11 PM
bjope added a comment to D56678: [DebugInfo][DAG] Allow creation of DBG_VALUEs in blocks where the operand is not used.

Interesting. If I remember correctly this is similar to something I played around with half a year ago (as a preparation before prohibiting CodeGenPrepare::placeDbgValues to move DBG_VALUE between basic blocks (or even to disable CodeGenPrepare::placeDbgValues completely)). I'll have a closer look at this when I'm at the office. I'll also try to remember if I had some specific reasons for not moving forward with my attempt at doing this - it has been bothering me that I never found the time to finalize that patch.

Jan 14 2019, 11:43 AM
bjope added inline comments to rL351050: [VFS] Allow multiple RealFileSystem instances with independent CWDs..
Jan 14 2019, 5:06 AM

Jan 11 2019

bjope added a comment to D56587: Introduce DW_OP_LLVM_convert.
In D56587#1354394, @vsk wrote:

Thanks for the patch.

t’s not obvious to me from skimming what the bug is with sign extension expressions. Could you describe what goes wrong, and maybe share a small program which shows the debugger behaving incorrectly?

Jan 11 2019, 9:36 AM · debug-info
bjope committed rC350933: Silence -Wsign-compare in unittests.
Silence -Wsign-compare in unittests
Jan 11 2019, 8:58 AM
bjope committed rL350933: Silence -Wsign-compare in unittests.
Silence -Wsign-compare in unittests
Jan 11 2019, 8:58 AM
bjope added inline comments to D56587: Introduce DW_OP_LLVM_convert.
Jan 11 2019, 8:30 AM · debug-info

Jan 10 2019

bjope committed rL350807: Fix RUN line in test/Transforms/LoopDeletion/crashbc.ll.
Fix RUN line in test/Transforms/LoopDeletion/crashbc.ll
Jan 10 2019, 2:02 AM

Dec 21 2018

bjope committed rCTE349891: Fix warning about unused variable [NFC].
Fix warning about unused variable [NFC]
Dec 21 2018, 12:55 AM
bjope committed rL349891: Fix warning about unused variable [NFC].
Fix warning about unused variable [NFC]
Dec 21 2018, 12:54 AM

Dec 14 2018

bjope added inline comments to D55625: [Intrinsic] Unsigned Fixed Point Multiplication Intrinsic.
Dec 14 2018, 6:53 AM · Restricted Project

Dec 4 2018

bjope added inline comments to D54719: [Intrinsic] Signed Fixed Point Multiplication Intrinsic.
Dec 4 2018, 6:57 AM

Nov 29 2018

bjope accepted D50977: [TableGen] Examine entire subreg compositions to detect ambiguity.

Might wait a little before you land this to see if @uweigand got anything more to say. But afaict this only removes the warning without affecting the result in any way, so it should not make anything worse.

Nov 29 2018, 7:48 AM
bjope added a comment to D50977: [TableGen] Examine entire subreg compositions to detect ambiguity.

Got a feeling that I'm the one who has been questioning this solution. And I guess I also kind of blocked the progress in D54962.
Anyway, I haven't really got any better alternative right now (at least not that is easy to implement).

Nov 29 2018, 7:16 AM

Nov 28 2018

bjope added inline comments to D54962: [SystemZ] Rework subreg structure to avoid TableGen warning.
Nov 28 2018, 3:04 AM

Nov 27 2018

bjope added inline comments to D50977: [TableGen] Examine entire subreg compositions to detect ambiguity.
Nov 27 2018, 6:53 AM

Nov 22 2018

bjope added reviewers for D54517: [CGP] Limit Complex Addressing mode by number of BasicBlocks to traverse: chandlerc, haicheng.

I don't have any objections to the patch myself, but I don't really have the knowledge to understand if this could be bad for some important use cases either. A user can ofcourse override the threshold if needed, so I guess that makes this patch a little bit less "dangerous".

Nov 22 2018, 2:07 PM
bjope added a comment to D54695: [PM] Port Scalarizer to the new pass manager..

return value for Scalarizer::run fixed in rL347432

Nov 22 2018, 12:53 AM

Nov 21 2018

bjope added a comment to D54719: [Intrinsic] Signed Fixed Point Multiplication Intrinsic.

Do we have examples of real hardware that implements this sort of instruction?

Right now this intrinsic looks like its just reimplementing what you would get if you just emitted this IR

%a = sext iX %x to i2X
%b = sext iX %y to i2X
%c = mul i2X %a, %b
%d = lshr i2X %c, PRECISION
%e = trunc i2X %d to iX

So I'm not sure if there's a need for an intrinsic unless that pattern is difficult to match to an instruction.

I think @ebevhan and @bjope may have hardware with fixed point instructions

Nov 21 2018, 12:58 PM
bjope added inline comments to D54695: [PM] Port Scalarizer to the new pass manager..
Nov 21 2018, 10:26 AM

Nov 20 2018

bjope added a comment to D54517: [CGP] Limit Complex Addressing mode by number of BasicBlocks to traverse.

Basically I think the code looks ok. However, I'm not so familiar with this algorithm so it is hard to comment about the actual solution.

Nov 20 2018, 2:37 PM
bjope added inline comments to D54719: [Intrinsic] Signed Fixed Point Multiplication Intrinsic.
Nov 20 2018, 4:29 AM
bjope added inline comments to D54729: [Docs] Documentation for the saturation addition and subtraction intrinsics.
Nov 20 2018, 3:41 AM
bjope accepted D54729: [Docs] Documentation for the saturation addition and subtraction intrinsics.

LGTM!

Nov 20 2018, 2:35 AM

Nov 14 2018

bjope added a comment to D54237: Constant folding and instcombine for saturating adds.

Please can you add vector constant folding tests + support?

Nov 14 2018, 6:16 AM

Nov 11 2018

bjope added a comment to D54237: Constant folding and instcombine for saturating adds.

Is it perhaps "better" to fold sadd_sat(X, undef) -> 0 And uadd_sat(X, undef) -> MaxValue if we want to get rid of undef here? That way we get rid of the X operand as well.

Folding sadd_sat(X, undef) -> 0 would not be valid, because if X = SignedMinValue, there is no choice of undef for which the result would be zero. The largest value that can be reached is -1.

Nov 11 2018, 3:04 AM
bjope accepted D54007: Use a data structure better suited for large sets in SimplificationTracker..

LGTM now (might wanna wait for @skatkov to take a look at the last changes as well).

Nov 11 2018, 2:52 AM

Nov 10 2018

bjope added a comment to D49671: [SchedModel] Propagate read advance cycles to implicit operands outside instruction descriptor.

Maybe the register allocator should add the implicit-def as an IMPLICIT_DEF instruction just before the MI instead of attaching a bogus impl def on the MI (if the MI uses parts of the register I guess the IMPLICIT_DEF needs a corresponding implicit use).

Nov 10 2018, 11:09 AM

Nov 9 2018

bjope added a comment to D54237: Constant folding and instcombine for saturating adds.

Regarding

// X + undef -> undef
// undef + X -> undef
if (match(Op1, m_Undef()) || match(Op0, m_Undef()))
  return UndefValue::get(ReturnType);

I was initially planning to include these simplifications, but ultimately was not certain regarding their legality. In particular, if we have uadd.sat(MaxValue, Y), then the result is fully determined to be MaxValue, regardless of the value of Y. If we have something like sadd.sat(SignedMinValue, Y) then the result is known to be negative. In either case the intrinsic cannot have the full range of results of the result type, regardless of the value of Y. As such, I think folding operations on undef to undef would not be legal in this case.

It should be possible to fold uadd.sat(X, undef) to MaxValue. Not sure how useful that is though.

You can also assume that undef is 0 and fold X + undef -> X.

Nov 9 2018, 10:49 AM

Nov 8 2018

bjope added inline comments to D54237: Constant folding and instcombine for saturating adds.
Nov 8 2018, 2:32 AM
bjope added a comment to D54237: Constant folding and instcombine for saturating adds.

Regarding

// X + undef -> undef
// undef + X -> undef
if (match(Op1, m_Undef()) || match(Op0, m_Undef()))
  return UndefValue::get(ReturnType);

I was initially planning to include these simplifications, but ultimately was not certain regarding their legality. In particular, if we have uadd.sat(MaxValue, Y), then the result is fully determined to be MaxValue, regardless of the value of Y. If we have something like sadd.sat(SignedMinValue, Y) then the result is known to be negative. In either case the intrinsic cannot have the full range of results of the result type, regardless of the value of Y. As such, I think folding operations on undef to undef would not be legal in this case.

It should be possible to fold uadd.sat(X, undef) to MaxValue. Not sure how useful that is though.

Nov 8 2018, 1:59 AM