tree-optimization/105937 - avoid uninit diagnostics crossing iterations

Message ID 20220819135557.3EE7F13AC1@imap2.suse-dmz.suse.de
State New, archived
Headers
Series tree-optimization/105937 - avoid uninit diagnostics crossing iterations |

Commit Message

Richard Biener Aug. 19, 2022, 1:55 p.m. UTC
  The following avoids adding PHIs to the worklist for uninit processing
if we reach them following backedges.  That confuses predicate analysis
because it assumes the use is happening in the same iteration as the the
definition.  For the testcase in the PR the situation is like

void foo (int val)
{
  int uninit;
  # val = PHI <..> (B)
  for (..)
    {
      if (..)
        {
          .. = val; (C)
          val = uninit;
        }
      # val = PHI <..> (A)
    }
}

and starting from (A) with 'uninit' as argument we arrive at (B)
and from there at (C).  Predicate analysis then tries to prove
the predicate of (B) (not the backedge) can prove that the
path from (B) to (C) is unreachable which isn't really what it
necessary - that's what we'd need to do when the preheader
edge of the loop were the edge with the uninitialized def.

So the following makes those cases intentionally false negatives.

Bootstrapped and tested on x86_64-unknown-linux-gnu, will push
on Monday if there are no comments.

Thanks,
Richard.

	PR tree-optimization/105937
	* tree-ssa-uninit.cc (find_uninit_use): Do not queue PHIs
	on backedges.
	(execute_late_warn_uninitialized): Mark backedges.

	* g++.dg/uninit-pr105937.C: New testcase.
---
 gcc/testsuite/g++.dg/uninit-pr105937.C | 235 +++++++++++++++++++++++++
 gcc/tree-ssa-uninit.cc                 |  14 +-
 2 files changed, 247 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/uninit-pr105937.C
  

Patch

diff --git a/gcc/testsuite/g++.dg/uninit-pr105937.C b/gcc/testsuite/g++.dg/uninit-pr105937.C
new file mode 100644
index 00000000000..26b4f74c5e1
--- /dev/null
+++ b/gcc/testsuite/g++.dg/uninit-pr105937.C
@@ -0,0 +1,235 @@ 
+// { dg-do compile }
+// { dg-require-effective-target c++17 }
+// { dg-options "-O2 -Wall" }
+
+#include <stdint.h>
+#include <optional>
+#include <string_view>
+
+using utf8 = char;
+
+enum
+{
+    FONT_SIZE_TINY = 2,
+    FONT_SIZE_SMALL = 0,
+    FONT_SIZE_MEDIUM = 1,
+    FONT_SIZE_COUNT = 3
+};
+
+constexpr const uint16_t FONT_SPRITE_GLYPH_COUNT = 224;
+
+enum class FontSpriteBase : int16_t
+{
+    MEDIUM_EXTRA_DARK = -2,
+    MEDIUM_DARK = -1,
+
+    TINY = FONT_SIZE_TINY * FONT_SPRITE_GLYPH_COUNT,
+    SMALL = FONT_SIZE_SMALL * FONT_SPRITE_GLYPH_COUNT,
+    MEDIUM = FONT_SIZE_MEDIUM * FONT_SPRITE_GLYPH_COUNT,
+};
+
+struct TTFSurface;
+
+class CodepointView
+{
+private:
+    std::string_view _str;
+
+public:
+    class iterator
+    {
+    private:
+        std::string_view _str;
+        size_t _index;
+
+    public:
+        iterator(std::string_view str, size_t index)
+            : _str(str)
+            , _index(index)
+        {
+        }
+
+        bool operator==(const iterator& rhs) const
+        {
+            return _index == rhs._index;
+        }
+        bool operator!=(const iterator& rhs) const
+        {
+            return _index != rhs._index;
+        }
+        char32_t operator*() const
+        {
+            return GetNextCodepoint(&_str[_index], nullptr);
+        }
+        iterator& operator++()
+        {
+            return *this;
+        }
+        iterator operator++(int)
+        {
+            auto result = *this;
+            if (_index < _str.size())
+            {
+                const utf8* nextch;
+                GetNextCodepoint(&_str[_index], &nextch);
+                _index = nextch - _str.data();
+            }
+            return result;
+        }
+
+        size_t GetIndex() const
+        {
+            return _index;
+        }
+
+        static char32_t GetNextCodepoint(const char* ch, const char** next);
+    };
+
+    CodepointView(std::string_view str)
+        : _str(str)
+    {
+    }
+
+    iterator begin() const
+    {
+        return iterator(_str, 0);
+    }
+
+    iterator end() const
+    {
+        return iterator(_str, _str.size());
+    }
+};
+
+struct InternalTTFFont;
+using TTF_Font = InternalTTFFont;
+struct TTFFontDescriptor
+{
+    const utf8* filename;
+    const utf8* font_name;
+    int32_t ptSize;
+    int32_t offset_x;
+    int32_t offset_y;
+    int32_t line_height;
+    int32_t hinting_threshold;
+    TTF_Font* font;
+};
+using codepoint_t = uint32_t;
+
+#define abstract = 0
+
+struct ITTF
+{
+    virtual ~ITTF() = default;
+    virtual TTFFontDescriptor* ttf_get_font_from_sprite_base(FontSpriteBase spriteBase) abstract;
+    virtual TTFSurface* ttf_surface_cache_get_or_add(TTF_Font* font, std::string_view text) abstract;
+};
+
+namespace OpenRCT2 {
+    struct IContext
+    {
+        virtual ~IContext() = default;
+
+        virtual ITTF* GetTTF() abstract;
+    };
+}
+
+static void ttf_draw_string_raw_ttf(OpenRCT2::IContext* context, std::string_view text)
+{
+    TTFFontDescriptor* fontDesc = context->GetTTF()->ttf_get_font_from_sprite_base(FontSpriteBase::MEDIUM_EXTRA_DARK);
+    if (fontDesc->font == nullptr)
+    {
+        return;
+    }
+
+    TTFSurface* surface = context->GetTTF()->ttf_surface_cache_get_or_add(fontDesc->font, text);
+    if (surface == nullptr)
+        return;
+}
+
+namespace UnicodeChar
+{
+    // Punctuation
+    constexpr char32_t leftguillemet = 0xAB;
+    constexpr char32_t rightguillemet = 0xBB;
+    constexpr char32_t german_quote_open = 0x201E;
+    constexpr char32_t quote_open = 0x201C;
+    constexpr char32_t quote_close = 0x201D;
+
+    // Dingbats
+    constexpr char32_t up = 0x25B2;
+    constexpr char32_t small_up = 0x25B4;
+    constexpr char32_t right = 0x25B6;
+    constexpr char32_t down = 0x25BC;
+    constexpr char32_t small_down = 0x25BE;
+    constexpr char32_t left = 0x25C0;
+    constexpr char32_t tick = 0x2713;
+    constexpr char32_t plus = 0x2795;
+    constexpr char32_t minus = 0x2796;
+
+    // Emoji
+    constexpr char32_t cross = 0x274C;
+    constexpr char32_t variation_selector = 0xFE0F;
+    constexpr char32_t eye = 0x1F441;
+    constexpr char32_t road = 0x1F6E3;
+    constexpr char32_t railway = 0x1F6E4;
+}; // namespace UnicodeChar
+
+
+static bool ShouldUseSpriteForCodepoint(char32_t codepoint)
+{
+    switch (codepoint)
+    {
+        case UnicodeChar::up:
+        case UnicodeChar::down:
+        case UnicodeChar::leftguillemet:
+        case UnicodeChar::tick:
+        case UnicodeChar::cross:
+        case UnicodeChar::right:
+        case UnicodeChar::rightguillemet:
+        case UnicodeChar::small_up:
+        case UnicodeChar::small_down:
+        case UnicodeChar::left:
+        case UnicodeChar::quote_open:
+        case UnicodeChar::quote_close:
+        case UnicodeChar::german_quote_open:
+        case UnicodeChar::plus:
+        case UnicodeChar::minus:
+        case UnicodeChar::variation_selector:
+        case UnicodeChar::eye:
+        case UnicodeChar::road:
+        case UnicodeChar::railway:
+            return true;
+        default:
+            return false;
+    }
+}
+
+void ttf_process_string_literal(OpenRCT2::IContext* context, std::string_view text)
+{
+    CodepointView codepoints(text);
+    std::optional<size_t> ttfRunIndex;
+    for (auto it = codepoints.begin(); it != codepoints.end(); it++)
+    {
+        auto codepoint = *it;
+        if (ShouldUseSpriteForCodepoint(codepoint))
+        {
+            if (ttfRunIndex.has_value())
+            {
+                // Draw the TTF run
+                auto len = it.GetIndex() - ttfRunIndex.value();  // { dg-bogus "may be used uninitialized" }
+                ttf_draw_string_raw_ttf(context, text.substr(ttfRunIndex.value(), len));
+                ttfRunIndex = std::nullopt;
+            }
+
+            // Draw the sprite font glyph
+        }
+        else
+        {
+            if (!ttfRunIndex.has_value())
+            {
+                ttfRunIndex = it.GetIndex();
+            }
+        }
+    }
+}
diff --git a/gcc/tree-ssa-uninit.cc b/gcc/tree-ssa-uninit.cc
index 06a19821e18..7074c9117b2 100644
--- a/gcc/tree-ssa-uninit.cc
+++ b/gcc/tree-ssa-uninit.cc
@@ -41,6 +41,7 @@  along with GCC; see the file COPYING3.  If not see
 #include "gimple-predicate-analysis.h"
 #include "domwalk.h"
 #include "tree-ssa-sccvn.h"
+#include "cfganal.h"
 
 /* This implements the pass that does predicate aware warning on uses of
    possibly uninitialized variables.  The pass first collects the set of
@@ -1191,8 +1192,16 @@  find_uninit_use (gphi *phi, unsigned uninit_opnds,
 
       basic_block use_bb;
       if (gphi *use_phi = dyn_cast<gphi *> (use_stmt))
-	use_bb = gimple_phi_arg_edge (use_phi,
-				      PHI_ARG_INDEX_FROM_USE (use_p))->src;
+	{
+	  edge e = gimple_phi_arg_edge (use_phi,
+					PHI_ARG_INDEX_FROM_USE (use_p));
+	  use_bb = e->src;
+	  /* Do not look for uses in the next iteration of a loop, predicate
+	     analysis will not use the appropriate predicates to prove
+	     reachability.  */
+	  if (e->flags & EDGE_DFS_BACK)
+	    continue;
+	}
       else
 	use_bb = gimple_bb (use_stmt);
 
@@ -1342,6 +1351,7 @@  execute_late_warn_uninitialized (function *fun)
   /* Mark all edges executable, warn_uninitialized_vars will skip
      unreachable blocks.  */
   set_all_edges_as_executable (fun);
+  mark_dfs_back_edges (fun);
 
   /* Re-do the plain uninitialized variable check, as optimization may have
      straightened control flow.  Do this first so that we don't accidentally