search for: isguaranteednottobeundeforpoison

...m. I agree > > with Wei that other developers may also get hit by the same issue and the > > cost of leaving this issue open for long can be very high to the > community. > > I can't speak for others, but I am fine with adding a workaround for > this. However, I suspect isGuaranteedNotToBeUndefOrPoison in > LoopUnswitch may regress other benchmarks. > Any other thoughts on a more minimal fix? Otherwise, it sounds like we can only try to find the fix that does the least damage. We can't just leave it broken given it's triggering even in llvm bootstraps now :( -------------- next par...

...88 # BB#2: # in Loop: Header=BB1_1 Depth=1 89 movq b(%rip), %rsi 90 addq %rax, %rsi 91 movq %rsi, c(%rip) 92 movq $3, i_hasval(%rip) 93 incq %rdx 94 xorl %esi, %esi 95 cmpq %rcx, %rdx 96 jl .LBB1_1 97 .LBB1_3: 98 retq ``` IMHO, enhancing `isGuaranteedNotToBeUndefOrPoison` and using it as a precondition in loop unswitching is not enough. undef (and poison) value can be stored into memory, and also be passed by a function argument. `isGuaranteedNotToBeUndefOrPoison` will virtually return `false` for all cases except the value is some integer constant. Sanjoy's su...

...3, i_hasval(%rip) >> >> 93 incq %rdx >> >> 94 xorl %esi, %esi >> >> 95 cmpq %rcx, %rdx >> >> 96 jl .LBB1_1 >> >> 97 .LBB1_3: >> >> 98 retq >> >> ``` >> >> >> >> IMHO, enhancing `isGuaranteedNotToBeUndefOrPoison` and using it as a >> >> precondition in loop unswitching is >> >> not enough. undef (and poison) value can be stored into memory, and >> >> also >> >> be passed by a function argument. >> >> `isGuaranteedNotToBeUndefOrPoison` will virtual...

...gt; 90 addq %rax, %rsi >> 91 movq %rsi, c(%rip) >> 92 movq $3, i_hasval(%rip) >> 93 incq %rdx >> 94 xorl %esi, %esi >> 95 cmpq %rcx, %rdx >> 96 jl .LBB1_1 >> 97 .LBB1_3: >> 98 retq >> ``` >> >> IMHO, enhancing `isGuaranteedNotToBeUndefOrPoison` and using it as a >> precondition in loop unswitching is >> not enough. undef (and poison) value can be stored into memory, and also >> be passed by a function argument. >> `isGuaranteedNotToBeUndefOrPoison` will virtually return `false` for all >> cases except the va...

...vid offline and he suggested a simple workaround > solution. For the case in MemorySSA.cpp, we have "if (a == b)" with b > being defined by a phi, and the phi has an undef operand. We can > recognize the simple pattern and give up loop unswitch for it. The > idea is not to use isGuaranteedNotToBeUndefOrPoison to prove > correctness, but just to rule out some patterns where such error may > appear. > > I admit the solution is far from ideal, but at least it is very simple > to implement, and serves to mitigate the immediate correctness issue > while avoiding performance regression. What...

...und > > solution. For the case in MemorySSA.cpp, we have "if (a == b)" > with b > > being defined by a phi, and the phi has an undef operand. We can > > recognize the simple pattern and give up loop unswitch for it. The > > idea is not to use isGuaranteedNotToBeUndefOrPoison to prove > > correctness, but just to rule out some patterns where such error may > > appear. > > > > I admit the solution is far from ideal, but at least it is very > simple > > to implement, and serves to mitigate the immediate correctness...

...t; compiling MemorySSA.cpp can be easily exposed by other changes, and it > is very time consuming to triage and extract the issue, so we like to > have some temporary solution, like to make loop unswitching more > conservative. If it sounds ok, I can work on it. My plan is to enhance > isGuaranteedNotToBeUndefOrPoison and use it as a precondition in loop > unswitching. > > Thanks, > Wei. > > > > On Mon, Jul 17, 2017 at 1:24 AM, Nuno Lopes <nlopes at microsoft.com> wrote: > > Cool, thanks for debugging this issue and letting us know. > > > > We have a few patches to...

Cool, thanks for debugging this issue and letting us know. We have a few patches to fix this issue: - Introduce freeze in IR: https://reviews.llvm.org/D29011 - Lowering freeze: https://reviews.llvm.org/D29014 - Fix loop unswitch: https://reviews.llvm.org/D29015 Bonus patches to recover perf: - Be less conservative in loop unswitching: https://reviews.llvm.org/D29016 - Instcombine support