Skip to content

Commit

Permalink
Another workaround for 'forwarding' issue
Browse files Browse the repository at this point in the history
Doesn't seem to be a forwarding issue at all, but rather gcc thinking that rank
can be 0 in places where it can't.

gcc11 doesn't flag these, but also fails to optimize as bench-at:Small/Small as
much.

* ra/expr.hh (Match): Revert to no fwd.
* ra/big.hh (CellBig), ra/small.hh (CellSmall): Remove diagnostic guards around
  len().
* ra/ply.hh (ply_ravel): Move guards here, where the confusion happens.
* ra/expr.hh (Match::check): Idem.
  • Loading branch information
lloda committed Nov 28, 2023
1 parent b3e71f6 commit c734252
Show file tree
Hide file tree
Showing 5 changed files with 8 additions and 8 deletions.
3 changes: 0 additions & 3 deletions ra/big.hh
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,7 @@ struct CellBig

consteval static rank_t rank() requires (ANY!=framer) { return framer; }
constexpr rank_t rank() const requires (ANY==framer) { return rank_frame(std::ssize(dimv), dspec); }
#pragma GCC diagnostic push // gcc 12.2 and 13.2 with RA_DO_CHECK=0 and -fno-sanitize=all
#pragma GCC diagnostic warning "-Warray-bounds"
constexpr dim_t len(int k) const { return dimv[k].len; } // len(0<=k<rank) or step(0<=k)
#pragma GCC diagnostic pop
constexpr static dim_t len_s(int k) { return ANY; }
constexpr dim_t step(int k) const { return k<rank() ? dimv[k].step : 0; }
constexpr void adv(rank_t k, dim_t d) { c.cp += step(k)*d; }
Expand Down
5 changes: 4 additions & 1 deletion ra/expr.hh
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,10 @@ struct Match<checkp, std::tuple<P ...>, mp::int_list<I ...>>
return false;
} else if constexpr (1==c) {
for (int k=0; k<rank(); ++k) {
#pragma GCC diagnostic push // gcc 12.2 and 13.2 with RA_DO_CHECK=0 and -fno-sanitize=all.
#pragma GCC diagnostic warning "-Warray-bounds"
dim_t ls = len(k);
#pragma GCC diagnostic pop
if (((k<ra::rank(std::get<I>(t)) && ls!=choose_len(std::get<I>(t).len(k), ls)) || ...)) {
RA_CHECK(!checkp, "Mismatch on axis ", k, " [", (std::array { std::get<I>(t).len(k) ... }), "].");
return false;
Expand All @@ -357,7 +360,7 @@ struct Match<checkp, std::tuple<P ...>, mp::int_list<I ...>>
}

constexpr
Match(P ... p_): t(RA_FWD(p_) ...) // [ra1]
Match(P ... p_): t(p_ ...) // [ra1]
{
// TODO Maybe on ply, would make checkp unnecessary, make agree_xxx() unnecessary.
if constexpr (checkp && !(has_len<P> || ...)) {
Expand Down
3 changes: 3 additions & 0 deletions ra/ply.hh
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,8 @@ ply_ravel(A && a, Early && early = Nop {})
// find outermost compact dim.
rank_t * ocd = order;
dim_t ss = a.len(*ocd);
#pragma GCC diagnostic push // gcc 12.2 and 13.2 with RA_DO_CHECK=0 and -fno-sanitize=all
#pragma GCC diagnostic warning "-Warray-bounds"
for (--rank, ++ocd; rank>0 && a.keep_step(ss, order[0], *ocd); --rank, ++ocd) {
ss *= a.len(*ocd);
}
Expand Down Expand Up @@ -203,6 +205,7 @@ ply_ravel(A && a, Early && early = Nop {})
}
}
}
#pragma GCC diagnostic pop
}


Expand Down
3 changes: 0 additions & 3 deletions ra/small.hh
Original file line number Diff line number Diff line change
Expand Up @@ -265,10 +265,7 @@ struct CellSmall
ctype c;

consteval static rank_t rank() { return framer; }
#pragma GCC diagnostic push // gcc 13.2
#pragma GCC diagnostic warning "-Warray-bounds"
constexpr static dim_t len(int k) { return dimv[k].len; } // len(0<=k<rank) or step(0<=k)
#pragma GCC diagnostic pop
constexpr static dim_t len_s(int k) { return len(k); }
constexpr static dim_t step(int k) { return k<rank() ? dimv[k].step : 0; }
constexpr void adv(rank_t k, dim_t d) { c.cp += step(k)*d; }
Expand Down
2 changes: 1 addition & 1 deletion test/bug83.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ main()
{
ra::TestRecorder tr(std::cout);
ra::Big<double, 2> A({4, 4}, ra::_1*10 + ra::_0 + 3);
A = ra::expr([](auto && a) { return a(0, 0); }, ra::iter<2>(A));
A = ra::expr([](auto && a) { return a(0, 0); }, ra::iter<2>(A)); // [ra1]
tr.info("A ", A).test_eq(3, A);
return tr.summary();
}

0 comments on commit c734252

Please sign in to comment.