Page MenuHomePhabricator

nlopes (Nuno Lopes)
User

Projects

User does not belong to any projects.

User Details

User Since
Oct 3 2013, 11:31 AM (363 w, 2 d)

Recent Activity

Yesterday

nlopes added inline comments to D87965: [InstCombine] replace phi values from unreachable blocks with 'undef'.
Sat, Sep 19, 8:43 AM · Restricted Project

Thu, Sep 10

nlopes added a comment to D87149: [InstCombine] erase instructions leading up to unreachable.

Ok, let me make it more concrete.
it seems we have 3 possible semantics:

  1. volatile accesses never trap, but rather trigger UB when the address is not dereferenceable
  2. they trap if the address is not dereferenceable
  3. they may trap regardless (i.e., they can never be removed). Alternatively we can state that the load/store address traces are externally observable and can't change
Thu, Sep 10, 4:35 AM · Restricted Project
nlopes added a comment to D87149: [InstCombine] erase instructions leading up to unreachable.

I would say let's write an RFC and see if there are other opinions. Also, @nlopes what does alive2 think of such a proposal?

Thu, Sep 10, 4:27 AM · Restricted Project

Sun, Sep 6

nlopes added a comment to D86815: [LangRef] Adjust guarantee for llvm.memcpy to also allow equal arguments..

FYI: I've just run Alive2 (already patched for this new semantics) on the test suite and no regressions reported.

Sun, Sep 6, 11:52 AM · Restricted Project

Tue, Sep 1

nlopes added a comment to D86815: [LangRef] Adjust guarantee for llvm.memcpy to also allow equal arguments..

Why would we change this? What's the point of having separate memcpy and memmove intrinsics?

I'm reading this as clang mis-uses llvm.memcpy when it probably should be using llvm.memmove

There is a longstanding assumption made by ~every compiler that memcpy(p, p, n) is safe. That's what we should be encoding here. We should not be removing all overlap restrictions.

Tue, Sep 1, 3:26 AM · Restricted Project

Aug 7 2020

nlopes added a comment to D85522: [Loads] Globals are dereferenceable, if offset + size <= sizeof(global)..

The reasoning looks good, though the case when offset + size overflows doesn't seem to be considered.

Aug 7 2020, 7:06 AM · Restricted Project

Jul 13 2020

nlopes added a comment to D83360: [InstSimplify] Remove select ?, undef, X -> X and select ?, X, undef -> X.

FYI InstSimplify doing distribution of undef is a known bug: https://bugs.llvm.org/show_bug.cgi?id=33165

Jul 13 2020, 2:13 AM · Restricted Project, Restricted Project

Jul 10 2020

nlopes added a comment to D83440: [InstSimplify] Re-enable select ?, undef, X -> X transform when X is provably not poison.

A question about this: We got a couple of dozen MSAN warnings in the Google codebase from the revision that removed these -- I'm guessing that perhaps what happened was that the undef in question was a use-of-uninitialized-value, and this optimization was hiding the use so the MSAN checks didn't trigger. Is re-enabling this going to make those MSAN warnings go away again, re-hiding this undefined behavior?

Jul 10 2020, 1:47 AM · Restricted Project

Jul 9 2020

nlopes updated subscribers of D83360: [InstSimplify] Remove select ?, undef, X -> X and select ?, X, undef -> X.

that's fine but I still don't understand why the counterexample to my version says %x2 in @src can be undef

If I'm understanding correctly, this reduces to something like the following:

define i32 @src() {

%x2 = freeze i32 undef
ret i32 %x2

}

define i32 @tgt() {

ret i32 undef

}

This seems a little suspect, yes.

Jul 9 2020, 2:18 PM · Restricted Project, Restricted Project
nlopes added inline comments to D83440: [InstSimplify] Re-enable select ?, undef, X -> X transform when X is provably not poison.
Jul 9 2020, 1:13 AM · Restricted Project

Jul 8 2020

nlopes added a comment to D83360: [InstSimplify] Remove select ?, undef, X -> X and select ?, X, undef -> X.

Alive does like this https://alive2.llvm.org/ce/z/yhibbe which is what I was going to implement.

Jul 8 2020, 3:18 PM · Restricted Project, Restricted Project
nlopes updated subscribers of D83360: [InstSimplify] Remove select ?, undef, X -> X and select ?, X, undef -> X.
Jul 8 2020, 1:36 AM · Restricted Project, Restricted Project
nlopes accepted D83360: [InstSimplify] Remove select ?, undef, X -> X and select ?, X, undef -> X.

Here's an end-to-end miscompilation: https://bugs.llvm.org/show_bug.cgi?id=31633

Jul 8 2020, 1:35 AM · Restricted Project, Restricted Project

Jul 2 2020

nlopes committed rG7f903873b8a9: DSE: fix builtin function recognition to take decl into account (authored by nlopes).
DSE: fix builtin function recognition to take decl into account
Jul 2 2020, 2:39 AM

Jun 27 2020

nlopes accepted D82316: [LangRef] Add `noundef` attribute to documentation.
Jun 27 2020, 1:21 AM · Restricted Project

Jun 25 2020

nlopes added inline comments to D82316: [LangRef] Add `noundef` attribute to documentation.
Jun 25 2020, 11:54 AM · Restricted Project

Jun 23 2020

nlopes added a comment to D81678: Introduce noundef attribute at call sites for stricter poison analysis.

I'm a bit concerned with this patch as it increases the amount of UB that LLVM exploits without any study of the impact.
For example, right now it's ok do this with clang (not with constants; make it less trivial so clang doesn't fold it right away):

int f() { return INT_MAX + 1; }
Jun 23 2020, 7:26 AM · Restricted Project, Restricted Project
nlopes added inline comments to D82316: [LangRef] Add `noundef` attribute to documentation.
Jun 23 2020, 7:26 AM · Restricted Project

May 31 2020

nlopes added a comment to D80875: [ValueTracking] Branch on poison is UB, branch cond is non-poison..

The passes we are aware that introduce branch on poison are: IndVarSimplify, LoopUnswitch, SimpleLoopUnswitch, and SimpifyCFG.
(https://web.ist.utl.pt/nuno.lopes/alive2/index.php?hash=4beb2b117e2fdd2c)

May 31 2020, 4:16 AM · Restricted Project

May 21 2020

nlopes added inline comments to D80276: [Alignment] Fix misaligned interleaved loads.
May 21 2020, 10:48 AM · Restricted Project

May 20 2020

nlopes added inline comments to D80276: [Alignment] Fix misaligned interleaved loads.
May 20 2020, 9:48 AM · Restricted Project
nlopes added inline comments to D80276: [Alignment] Fix misaligned interleaved loads.
May 20 2020, 7:02 AM · Restricted Project
nlopes updated subscribers of D80276: [Alignment] Fix misaligned interleaved loads.
May 20 2020, 7:02 AM · Restricted Project

May 11 2020

nlopes updated subscribers of D79746: [SCEV] Relax abnormal exit check in isAddRecNeverPoison..
May 11 2020, 2:35 PM · Restricted Project

May 8 2020

nlopes added inline comments to D78615: [ValueTracking] Let propagatesPoison support binops/unaryops/cast/etc..
May 8 2020, 10:10 AM · Restricted Project

Apr 22 2020

nlopes added a comment to D78631: [InstCombine] intersect FMF when reassociating FP min/max intrinsics.

Side note - Alive2 does not seem to recognize the keyword 'fast' currently (or we would have seen more tests flagged as incorrect).

Apr 22 2020, 8:06 AM · Restricted Project

Apr 13 2020

nlopes accepted D77868: [InstSimplify] fold select of bools using bitwise logic.
// select Cond, T, false --> Cond & T
if (match(F, m_ZeroInt()))
  return SimplifyAndInst(Cond, T, Q);

I still do not see how this can go wrong in practice. If InstSimplify can prove that T is poison, then doesn't it always manifest that knowledge by saying T is undef?
Looking at it another way (see if you can spot a logic flaw):

  1. Assume T is poison.
  2. Assume InstSimplify fails to simplify T to undef or constant.
  3. For poison T' (either T or some other poisoned value) to leak through as the result, InstSimplify must prove that Cond & T' == T'.
  4. So InstSimplify must prove that Cond == true or Cond == T'.
  5. If Cond == T', there's no problem (if Cond is poison, the select is poison).
  6. Therefore -- for there to be a problem -- we must prove that Cond is true.
  7. If we can prove that Cond is true, then that guarantees that value T' can't depend on Cond (nothing depends on Cond - it simplifies to constant true).
Apr 13 2020, 9:06 AM

Apr 5 2020

nlopes added a comment to D76973: [LangRef] Clarify the semantics of branch on undef.

Yes, introducing branches on a variable that may be undef/poison is not legal. However, you can use freeze to make it safe.
I think @aqjune fixed loop unswitching already. (don't recall if that was the reverted patch). It's true there a couple more places left to fix.

Apr 5 2020, 2:57 PM · Restricted Project

Mar 29 2020

nlopes added a comment to D76973: [LangRef] Clarify the semantics of branch on undef.

We should probably also think about what to do with all the tests that contain branches on undef. Also, bugpoint should probably stop creating branches on undef with this spelled out as is?

When I've updated other tests that would break with changed/improved undef semantics, I usually replace the 'undef' with a parameter like this:
rGfebcb24f1490
Maybe we can script that to fix most of the tests?

Mar 29 2020, 12:17 PM · Restricted Project

Mar 28 2020

nlopes added inline comments to D76973: [LangRef] Clarify the semantics of branch on undef.
Mar 28 2020, 12:22 PM · Restricted Project
nlopes accepted D76973: [LangRef] Clarify the semantics of branch on undef.
Mar 28 2020, 12:22 PM · Restricted Project

Feb 29 2020

nlopes accepted D75396: [ValueTracking] A value is never undef or poison if it must raise UB.

LGTM

Feb 29 2020, 9:05 AM · Restricted Project
nlopes accepted D75397: [ValueTracking] Let getGuaranteedNonFullPoisonOp consider assume, remove mentioning about br.

LGTM (modulo the style fix).
assume poison is UB.

Feb 29 2020, 8:56 AM · Restricted Project

Feb 23 2020

nlopes committed rG98ac6e76960a: [NFC] fix test nan value (authored by nlopes).
[NFC] fix test nan value
Feb 23 2020, 4:47 AM

Feb 21 2020

nlopes added a comment to D74935: [LangRef][AliasAnalysis] Clarify `noalias` affects only modified objects.

I agree with @aqjune that stating clearly the definition of object in this context.
See this example in the C spec:

For example, the second call of f in g has undefined behavior because each of d[1] through d[49] is accessed through both p and q.
Feb 21 2020, 3:23 AM · Restricted Project

Feb 17 2020

nlopes added inline comments to D74713: [ConstantFold] fold fsub -0.0, undef to undef rather than NaN.
Feb 17 2020, 2:36 PM · Restricted Project

Feb 14 2020

nlopes added a comment to D74209: [AssumeBundle] Add documentation for the operand bundles of an llvm.assume.

LGTM overall, just some nitpicks.
Something I would add is a link to the list of attributes (is this function-attributes?).

Feb 14 2020, 9:54 AM · Restricted Project

Feb 7 2020

nlopes committed rG380fe91fc6dd: [docs] update mathjax path in doxygen (authored by nlopes).
[docs] update mathjax path in doxygen
Feb 7 2020, 8:27 AM

Jan 24 2020

nlopes added a comment to D73342: Fix EarlyCSE to intersect aliasing metadata..

I'm not familiar with the code of this pass, but is there a cheap way of identifying that the two operations are in the same basic block?
If so, you could take the intersection of the aliasing information rather than the union. Because if both ops are guaranteed to execute then the tightest aliasing still has to hold.
(being in the same BB doesn't imply that both instructions are executed, but there's code in ValueTracking perhaps that can check that)

Jan 24 2020, 8:13 AM · Restricted Project

Jan 11 2020

nlopes committed rG87407fc03c82: DSE: fix bug where we would only check libcalls for name rather than whole decl (authored by nlopes).
DSE: fix bug where we would only check libcalls for name rather than whole decl
Jan 11 2020, 3:59 AM

Jan 9 2020

nlopes added a comment to D29014: [SelDag] Add FREEZE.

The patch & semantics look good to me, but I'm not a backend expert. I'll leave the final LGTM to someone else.
It would be awesome if we could get this in for 10.0 so that we have complete support for freeze.

Jan 9 2020, 6:27 AM · Restricted Project

Jan 3 2020

nlopes added a comment to D71499: Add builtins for aligning and checking alignment of pointers and integers.

@lebedev.ri I agree with you that the semantics of these alignment builtins should only return a pointer that is of the same object as the one given as input.
Otherwise, these builtins would be even worst that ptr2int/int2ptr, since their result could alias with any other pointer in the program, not just the escaped pointers.

Jan 3 2020, 9:18 AM · Restricted Project

Jan 2 2020

nlopes accepted D72101: [InstCombine] replace undef elements in vector constant when doing icmp folds (PR44383).

LGTM, thanks!

Jan 2 2020, 1:55 PM · Restricted Project

Dec 10 2019

nlopes added a comment to D71145: [InstCombine] Allow to limit the max number of iterations.

LGTM. The table you provided was very interesting to see, thanks!

Dec 10 2019, 2:18 PM · Restricted Project

Dec 4 2019

nlopes added a comment to D70749: [InstCombine] do not insert nonnull assumption for undef.

You said "Changing the semantics on non-respecting the tags from UB to poison doesn't help either.", could you elaborate why?
If there are no uses of a violating instantiation (undef/null passed to a non-null) we would not get UB but an unused poison value.

Dec 4 2019, 1:39 AM · Restricted Project

Dec 1 2019

nlopes committed rG89c47313c9b1: remove UB from test by making GV alignment explicit (authored by nlopes).
remove UB from test by making GV alignment explicit
Dec 1 2019, 7:20 AM

Nov 29 2019

nlopes added a comment to D70749: [InstCombine] do not insert nonnull assumption for undef.

I think it's clear that dead arg elimination is incorrect in replacing a valid pointer with null in an attributed with non-null tag. Changing the semantics on non-respecting the tags from UB to poison doesn't help either.
The problem with dropping attributes is that a given function call site, the attributes to be considered are the union of the attributes in the function call and in the callee declaration. We can't rely drop attributes from the callee since some linking later may add them back.

Nov 29 2019, 9:30 AM · Restricted Project
nlopes accepted D70641: [LangRef] make per-element poison behavior explicit.

LGTM
Reference to the discussion: http://lists.llvm.org/pipermail/llvm-dev/2019-November/137243.html

Nov 29 2019, 6:49 AM · Restricted Project

Nov 24 2019

nlopes added a comment to D70641: [LangRef] make per-element poison behavior explicit.

The first bit looks ok to me, thanks!

Nov 24 2019, 2:32 PM · Restricted Project
nlopes added a comment to D70623: [SCEV] Compute trip counts w/frozen conditions.

Interesting question :)

Nov 24 2019, 5:06 AM · Restricted Project

Nov 18 2019

nlopes added a comment to D70246: [InstCombine] remove identity shuffle simplification for mask with undefs.

Let me give my 2c: eliminating this transformation is a good thing, since it's incorrect (end-to-end miscompilation testcase in the cited bug report).
It can be re-instated if/when we switch from initializing vectors with undef with poison, but I don't know when that will happen. We'll try to push for the poison patches soonish, but it will take time.
Anyway, thanks Sanjay for handling this :)

Nov 18 2019, 6:39 AM · Restricted Project

Nov 11 2019

nlopes committed rGa7244c56bdd0: docs: fix warning in LangRef parsing (authored by nlopes).
docs: fix warning in LangRef parsing
Nov 11 2019, 2:47 AM
nlopes added a comment to D29121: [Docs] Add LangRef documention for freeze instruction.

@nlopes This is failing on the llvm-sphinx-docs buildbot, please can you take a look? http://lab.llvm.org:8011/builders/llvm-sphinx-docs/builds/37793/steps/docs-llvm-html/logs/stdio

Warning, treated as error:
/home/buildbot/llvm-build-dir/llvm-sphinx-docs/llvm/src/llvm/docs/LangRef.rst:10243: WARNING: Could not lex literal_block as "llvm". Highlighting skipped.
Nov 11 2019, 2:44 AM · Restricted Project

Nov 6 2019

nlopes added a comment to D29011: [IR] Add Freeze instruction.

Does a ConstantExpr freeze really make sense? ConstantExprs are uniqued by the LLVMContext. So every constantexpr that has the same input ends up being the same object. Do we want that behavior do we need each freeze to be distinct?

Nov 6 2019, 2:40 AM · Restricted Project

Nov 5 2019

nlopes committed rG2d21068d9fa0: [Docs] Add LangRef documentation for freeze instruction (authored by nlopes).
[Docs] Add LangRef documentation for freeze instruction
Nov 5 2019, 3:40 AM
nlopes closed D29121: [Docs] Add LangRef documention for freeze instruction.
Nov 5 2019, 3:40 AM · Restricted Project

Nov 2 2019

nlopes accepted D69690: [GlobalsAA] Restrict ModRef result if any internal method has its address taken..

LGTM!

Nov 2 2019, 12:56 PM · Restricted Project

Oct 30 2019

nlopes added a comment to D29121: [Docs] Add LangRef documention for freeze instruction.

So this is not committed till we reviewed all of them?

I'm just waiting on SelDAG->MIR patch to be done (https://reviews.llvm.org/D29014).
Then we have docs, SelDAG & MIR support. That seems the minimal to me (so the compiler won't crash when freeze is used). I agree the optimizer patches can land later.

Can I suggest that we add a stub* ISEL implementation, land this, and then build on top of it?

  • By which I mean a knowingly incorrect lowering to a simple COPY. It wouldn't fix *all* of our poison/undef problems, but it would allow us to make progress on some of the most painful IR ones while waiting for that patch to land.
Oct 30 2019, 6:39 PM · Restricted Project

Oct 28 2019

nlopes added a comment to D29121: [Docs] Add LangRef documention for freeze instruction.

So this is not committed till we reviewed all of them?

Oct 28 2019, 6:18 PM · Restricted Project

Oct 25 2019

nlopes added a comment to D69442: [CVP] prevent propagating poison when substituting edge values into a phi (PR43802).

LGTM

Oct 25 2019, 3:43 PM · Restricted Project

Oct 20 2019

nlopes committed rL375358: add John Regehr as speaker to Alive2 talk.
add John Regehr as speaker to Alive2 talk
Oct 20 2019, 2:29 AM

Oct 16 2019

nlopes added a comment to D68342: [Analysis] Don't assume that unsigned overflow can't happen in EmitGEPOffset (PR42699).

LGTM
(there's only a typo in the comment "singned")

Oct 16 2019, 12:21 PM · Restricted Project

Oct 15 2019

nlopes added a comment to D68342: [Analysis] Don't assume that unsigned overflow can't happen in EmitGEPOffset (PR42699).

The patch looks good to me. Actually I had reported this bug a while back as well: https://bugs.llvm.org/show_bug.cgi?id=42699
I agree we can't have objects larger than half of the address space.

Oct 15 2019, 8:56 AM · Restricted Project

Oct 7 2019

nlopes updated the diff for D29121: [Docs] Add LangRef documention for freeze instruction.

Clarify semantics for pointers.

Oct 7 2019, 3:17 PM · Restricted Project

Oct 5 2019

nlopes added a comment to D29121: [Docs] Add LangRef documention for freeze instruction.

@nlopes anything holding this patch? do you intend to land them all at once?
@aqjune I suppose D29011 should be the next one up for review..

Oct 5 2019, 6:20 AM · Restricted Project

Sep 17 2019

nlopes added inline comments to D29121: [Docs] Add LangRef documention for freeze instruction.
Sep 17 2019, 6:35 AM · Restricted Project

Sep 16 2019

nlopes updated the diff for D29121: [Docs] Add LangRef documention for freeze instruction.

Made it explicit re one vs multiple calls to freeze.

Sep 16 2019, 6:45 AM · Restricted Project
nlopes added inline comments to D29121: [Docs] Add LangRef documention for freeze instruction.
Sep 16 2019, 5:25 AM · Restricted Project
nlopes updated the diff for D29121: [Docs] Add LangRef documention for freeze instruction.

Updated with an example with vectors and add more references to freeze section.

Sep 16 2019, 4:18 AM · Restricted Project
nlopes updated the diff for D29121: [Docs] Add LangRef documention for freeze instruction.

Reflect comments & clarify immediate UB when branching on poison.

Sep 16 2019, 2:23 AM · Restricted Project

Aug 31 2019

nlopes committed rL370568: add nlopes.
add nlopes
Aug 31 2019, 2:23 AM

Jul 10 2019

nlopes added a comment to D64215: Add a transform pass to make the executable semantics of poison explicit in the IR.

Just a shameless plug :)
We've been half secretly working on Alive2 (https://github.com/AliveToolkit/alive2), which includes a plugin for opt that can check if an optimization is correct or not. Alive2 also has a standalone tool that accepts 2 IR files instead.

I'd tried playing with Alive2 a while ago, and had trouble getting it to work. Could you maybe update the readme (or other docs) with some instructions on how to use the standalone tool you mentioned? I'd very much like to play with this.

I've just added a short description to the README file: https://github.com/AliveToolkit/alive2#running-standalone-translation-validation-tool-alive-tv
It's fairly simple: it just takes 2 LLVM IR files. Let me know if you have questions or if you find bugs :)

This tool implements the semantics of poison for many LLVM instructions, and already has some support for memory (which is quite hard to handle).
Of course, what this patch does is not the same. This patch is more executable, while Alive2 requires Z3 to reason about the semantics (though it can also execute code very slowly).

I'd love to explore options for sharing the semantics here. What form does Alive2 express them in?

That's still a unsolved research problem. No one really knows how to share semantics still.
The semantics in Alive2 are written in C++, using an embedded expression language. While it is potentially possible to reuse that somewhere else, it isn't trivial. See e.g. the ir/instr.cpp file.

One thought on sharing.

From what I can tell from a quick look at the code you mentioned, it looks like you're parsing IR into an expression language, then rewriting the expressions to propagate poison - in a fairly similar manner to this code, but over your expression language - and then translating that expression language to SMT. Is that a good high level summary?

Jul 10 2019, 9:43 AM · Restricted Project
nlopes added inline comments to D64451: [PoisonChecking] Validate inbounds annotation on getelementptr where possible.
Jul 10 2019, 9:34 AM · Restricted Project

Jul 9 2019

nlopes added a comment to D64215: Add a transform pass to make the executable semantics of poison explicit in the IR.

Just a shameless plug :)
We've been half secretly working on Alive2 (https://github.com/AliveToolkit/alive2), which includes a plugin for opt that can check if an optimization is correct or not. Alive2 also has a standalone tool that accepts 2 IR files instead.

I'd tried playing with Alive2 a while ago, and had trouble getting it to work. Could you maybe update the readme (or other docs) with some instructions on how to use the standalone tool you mentioned? I'd very much like to play with this.

I've just added a short description to the README file: https://github.com/AliveToolkit/alive2#running-standalone-translation-validation-tool-alive-tv
It's fairly simple: it just takes 2 LLVM IR files. Let me know if you have questions or if you find bugs :)

I finally got it working, required a couple changes to the CMakeFiles and an LD_PRELOAD (for unclear reasons). However, it doesn't look like the scope of the alive-tv tool is anywhere near wide enough for my purposes.

Correct me if I'm wrong, but it looks like it can't handle loops at all right? And use of any memory seams to trigger timeouts? (Even for trivially identical IR?) Just making sure there's no error between keyboard and chair. :)

Jul 9 2019, 12:09 PM · Restricted Project

Jul 6 2019

nlopes added a comment to D64215: Add a transform pass to make the executable semantics of poison explicit in the IR.

Just a shameless plug :)
We've been half secretly working on Alive2 (https://github.com/AliveToolkit/alive2), which includes a plugin for opt that can check if an optimization is correct or not. Alive2 also has a standalone tool that accepts 2 IR files instead.

I'd tried playing with Alive2 a while ago, and had trouble getting it to work. Could you maybe update the readme (or other docs) with some instructions on how to use the standalone tool you mentioned? I'd very much like to play with this.

Jul 6 2019, 3:37 AM · Restricted Project

Jul 5 2019

nlopes added a comment to D64215: Add a transform pass to make the executable semantics of poison explicit in the IR.

Just a shameless plug :)
We've been half secretly working on Alive2 (https://github.com/AliveToolkit/alive2), which includes a plugin for opt that can check if an optimization is correct or not. Alive2 also has a standalone tool that accepts 2 IR files instead.
This tool implements the semantics of poison for many LLVM instructions, and already has some support for memory (which is quite hard to handle).
Of course, what this patch does is not the same. This patch is more executable, while Alive2 requires Z3 to reason about the semantics (though it can also execute code very slowly).

Jul 5 2019, 6:50 AM · Restricted Project

Jun 10 2019

nlopes added a comment to D63044: [LangRef] Clarify poison semantics.

LGTM, thank you!

Jun 10 2019, 9:55 AM · Restricted Project

Jun 9 2019

nlopes added inline comments to D63044: [LangRef] Clarify poison semantics.
Jun 9 2019, 11:25 AM · Restricted Project
nlopes updated subscribers of D63044: [LangRef] Clarify poison semantics.
Jun 9 2019, 2:55 AM · Restricted Project
nlopes added a comment to D63044: [LangRef] Clarify poison semantics.

Sounds great! Just 1 comment inline.

Jun 9 2019, 2:53 AM · Restricted Project

Mar 30 2019

nlopes accepted D57983: [ConstantRange] Shl of ConstantRanges considers full-set shifting to last bit position.

LGTM.
Minor: you have a typo in the comment: "return return"

Mar 30 2019, 3:06 AM · Restricted Project

Mar 29 2019

nlopes added a comment to D57983: [ConstantRange] Shl of ConstantRanges considers full-set shifting to last bit position.

I believe the contains function I wrote is correct. It says that an integer n belongs to interval I iff n >= lower(I) and n < upper(I) is there's no wrapping.
BTW, function isWrappedSet() just changed recently, so that needs tweaking in the Z3 model I sent (https://github.com/llvm-mirror/llvm/blob/master/lib/IR/ConstantRange.cpp#L347)

Mar 29 2019, 7:42 AM · Restricted Project

Mar 3 2019

nlopes requested changes to D57983: [ConstantRange] Shl of ConstantRanges considers full-set shifting to last bit position.

This patch is not correct for this input:
lhs = FullSet
rhs = [0, 1)

Mar 3 2019, 8:14 AM · Restricted Project

Feb 6 2019

nlopes committed rL353349: move attribute inference to project to gsoc 2019.
move attribute inference to project to gsoc 2019
Feb 6 2019, 2:35 PM

Dec 31 2018

nlopes added a comment to D56155: [docs] cttz and ctlz return poison, not undef, when argument is 0.

Based on the current documentation of poison values, selecting over poison is still poison by the first rule. I'm assuming that this is an inaccuracy in the documentation and selects are treated the same ways as phis? Otherwise the cited clang code would be affected.

Dec 31 2018, 2:03 AM · Restricted Project

Dec 30 2018

nlopes added a comment to D56155: [docs] cttz and ctlz return poison, not undef, when argument is 0.

As a justification why it doesn't matter whether it's undef or poison, clang emits__builtin_ffs as:
ffs(x) -> x ? cttz(x) + 1 : 0

Dec 30 2018, 6:44 AM · Restricted Project

Dec 29 2018

nlopes added reviewers for D56155: [docs] cttz and ctlz return poison, not undef, when argument is 0: sanjoy, spatel.
Dec 29 2018, 3:03 PM · Restricted Project

Dec 2 2018

nlopes committed rL348092: add our OOPLSA'18 paper.
add our OOPLSA'18 paper
Dec 2 2018, 8:00 AM

Dec 1 2018

nlopes committed rL348077: fix html.
fix html
Dec 1 2018, 7:16 AM

Nov 8 2018

nlopes 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, 2:41 PM

Sep 10 2018

nlopes added a comment to D51738: add IR flags to MI.

Sure, adding nsw/nuw brings in poison. I didn't study all the MI transformations going on, so I can't comment on whether this is a big burden or not, but without such a study introducing poison is dangerous.
SDAG already has nsw/nuw IIRC, though. But I don't think there was a thorough study of the implications at the time either.

Sep 10 2018, 3:14 AM

Jul 17 2018

nlopes committed rL337307: devmtg201810: add SRC PC members.
devmtg201810: add SRC PC members
Jul 17 2018, 10:55 AM

Jul 9 2018

nlopes committed rL336535: update next conf date.
update next conf date
Jul 9 2018, 3:23 AM

Jul 8 2018

nlopes added a comment to D49042: [LangRef] Clarify alloca of zero bytes..

Does this mean we can "construct" undef as:

x = alloca 0
y = alloca 0
undef = x == y  // Non-deterministically 0 or 1

? If so, this is a problem since it means even if we spec un-initialized memory as poison we still have undef in the IR (with all the problems it brings).

Jul 8 2018, 12:16 PM
nlopes added a comment to D49047: [InstCombine] fix shuffle-of-binops transform to avoid poison/undef .

BTW, I couldn't figure if this case is possible, but mentioning it just in case: it's also not legal to introduce 'sdiv undef, ..', since 'sdiv INT_MIN, -1' is UB. Same for srem. So we need to be careful when folding the shuffle on LHS as well.

Jul 8 2018, 11:10 AM
nlopes added inline comments to D49041: [LangRef] Clarify undefined behavior for function attributes..
Jul 8 2018, 8:05 AM

Jul 6 2018

nlopes added inline comments to D48987: [InstCombine] drop poison flags for shuffle transforms with undefs.
Jul 6 2018, 1:12 AM

Jul 5 2018

nlopes added inline comments to D48987: [InstCombine] drop poison flags for shuffle transforms with undefs.
Jul 5 2018, 2:32 PM

Jul 4 2018

nlopes added a comment to D48893: [Constants, InstCombine] allow RHS (operand 1) identity constants for binops.

shl %x, undef is poison; we don't want more undefs :)
So this transformation for shifts is not correct. The way to make it correct would be to introduce a poison value and use it in the shuffle instructions instead of undef. I suggest we finally go ahead and do that.

  1. Does the 'undef' in the shuffle mask represent something different than the 'undef' in the shift amount?
Jul 4 2018, 3:36 PM
nlopes added a comment to D48893: [Constants, InstCombine] allow RHS (operand 1) identity constants for binops.

shl %x, undef is poison; we don't want more undefs :)
So this transformation for shifts is not correct. The way to make it correct would be to introduce a poison value and use it in the shuffle instructions instead of undef. I suggest we finally go ahead and do that.

Jul 4 2018, 1:51 AM