[llvm-bugs] [Bug 47984] New: Optimizer generating wrong code

[via llvm-bugs]

https://bugs.llvm.org/show_bug.cgi?id=47984

            Bug ID: 47984
           Summary: Optimizer generating wrong code
           Product: clang
           Version: trunk
          Hardware: Macintosh
                OS: MacOS X
            Status: NEW
          Severity: normal
          Priority: P
         Component: LLVM Codegen
          Assignee: unassignedclangbugs at nondot.org
          Reporter: tbruio at outlook.com
                CC: llvm-bugs at lists.llvm.org, neeilans at live.com,
                    richard-llvm at metafoo.co.uk

Version
==========
Tested Clang 10 through 12.


Issue
==========
While running tests for Blender, we encountered an intermittent bug on binaries
compiled with clang.
It shows up in -O2 builds, not -O1, or -O0.

The code inside the for-loop at [1] is not executed with -O2. That causes more
problems down the line.

We fixed it by marking `last_remapped_id` volatile. Another way was to access
other fields of `last_remapped_id`.

Steps to redo:
==========
- git clone https://git.blender.org/blender.git
- cd blender
- make update
- git revert 2ddecfffc3d3 --no-commit
- cmake -B ../blender_build -C build_files/cmake/config/blender_lite.cmake -G
Ninja -DCMAKE_BUILD_TYPE=RelWithDebInfo
- cd ../blender_build  && make install
- ctest -R id_man -C relwithdebinfo -VV

The test will fail.

Diagnostics
==========
It has been been very difficult to isolate the bug to a simple code. Even
moving out the function `id_delete` to
another file fixes the bug.
There is a possibility that the following might be the reason of the bug
(quoting from https://developer.blender.org/D9315#230626)
"""
Just for context, repeating what I said earlier in the chat - this is what **I
think** goes on:
* The crash makes sense to me now, the "MECube" (which is the default cube from
the factory-startup I think) is tagged properly for deletion, but not the
"OBCube" - because the loop isn't executed correctly.
* The optimizer realizes that `tagged_deleted_ids` is collected the same way on
each iteration (which is based on `ID.tag`) and it operates on the same data,
so it assumes it will always yield the same result.
* It does not realise however that `ID.tag` is changed within the loop,
somewhere deeper down the call-stack (here in fact
https://developer.blender.org/diffusion/B/browse/master/source/blender/blenkernel/intern/lib_remap.c$158).

Note that this should be reported to the Clang team. The Godbolt links we used
in the chat will hopefully be enough, although if my analysis above is correct,
the code on Godbolt won't show the error, as the part of `ID.tag` being changed
indirectly in the loop is missing. Maybe a simple (non-inline!) function that
just modifies `ID.tag` will be enough.
"""

Attachments
===========
Four files are attached:
With volatile assembly
With volatile optimization record
Without volatile assembly
Without volatile optimization record

I can run more tests if needed.

[1]
https://developer.blender.org/diffusion/B/browse/master/source/blender/blenkernel/intern/lib_id_delete.c;2ddecfffc3d3a3a1db4ae45e8665caa2a85ab43a$293-310

if you prefer github: https://github.com/blender/blender

-- 
You are receiving this mail because:
You are on the CC list for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-bugs/attachments/20201026/0d27bfcd/attachment.html>