Skip to content

Commit 3889101

Browse files
JeffreyALawJeffreyALaw
committed
[rtl-optimization/116244] Don't create bogus regs in alter_subreg
> Jeff Law <[email protected]> writes: >> So pulling on this thread leads me into the code that sets up >> ALLOCNO_WMODE in create_insn_allocnos: >> >>> if ((a = ira_curr_regno_allocno_map[regno]) == NULL) >>> { >>> a = ira_create_allocno (regno, false, ira_curr_loop_tree_node); >>> if (outer != NULL && GET_CODE (outer) == SUBREG) >>> { >>> machine_mode wmode = GET_MODE (outer); >>> if (partial_subreg_p (ALLOCNO_WMODE (a), wmode)) >>> ALLOCNO_WMODE (a) = wmode; >>> } >>> } >> Note how we only set ALLOCNO_MODE only at allocno creation, so it'll >> work as intended if and only if the first reference is via a SUBREG. > > Huh, yeah, I agree that that looks wrong. > >> ISTM the fix here is to always do the check and set ALLOCNO_WMODE. >>[ Snipped discussion on a non-issue. ] > > So ISTM that moving the code out of the "if (... == NULL)" should be > enough on its own. > >> And it all makes sense that you caught this. You and another colleague >> at ARM were trying to address this exact problem ~11 years ago ;-) > > Heh, thought it sounded familiar :) So attached is the updated patch that adjusts IRA to avoid this problem. Georg-Johann, this may explain an issue you were running into as well where you got an invalid allocation. I think yours was at the higher end of the register file, but the core issue is potentially the same (looking at the first use rather than all of them for paradoxical subregs). I've had this in my tester about a week. So it's been through the crosses as well as various native bootstraps, including but not limited to m68k, ppc, s390, hppa, sh4, etc. And just for good measure I bootstrapped & regression tested it on x86_64 a few minutes ago. Pushing to the trunk. PR rtl-optimization/116244 gcc/ * ira-build.cc (create_insn_allocnos): Do not restrict the check for subreg uses to allocno creation time. Do it for all uses. gcc/testsuite/ * g++.target/m68k/m68k.exp: New test driver. * g++.target/m68k/pr116244.C: New test.
1 parent ca7c6d1 commit 3889101

File tree

3 files changed

+268
-7
lines changed

3 files changed

+268
-7
lines changed

‎gcc/ira-build.cc‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1851,14 +1851,15 @@ create_insn_allocnos (rtx x, rtx outer, bool output_p)
18511851
ira_allocno_t a;
18521852

18531853
if ((a = ira_curr_regno_allocno_map[regno]) == NULL)
1854+
a = ira_create_allocno (regno, false, ira_curr_loop_tree_node);
1855+
1856+
/* This used to only trigger at allocno creation which seems
1857+
wrong. We care about the WMODE propery across all the uses. */
1858+
if (outer != NULL && GET_CODE (outer) == SUBREG)
18541859
{
1855-
a = ira_create_allocno (regno, false, ira_curr_loop_tree_node);
1856-
if (outer != NULL && GET_CODE (outer) == SUBREG)
1857-
{
1858-
machine_mode wmode = GET_MODE (outer);
1859-
if (partial_subreg_p (ALLOCNO_WMODE (a), wmode))
1860-
ALLOCNO_WMODE (a) = wmode;
1861-
}
1860+
machine_mode wmode = GET_MODE (outer);
1861+
if (partial_subreg_p (ALLOCNO_WMODE (a), wmode))
1862+
ALLOCNO_WMODE (a) = wmode;
18621863
}
18631864

18641865
ALLOCNO_NREFS (a)++;

‎gcc/testsuite/g++.target/m68k/m68k.exp‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
# Copyright (C) 2019-2024 Free Software Foundation, Inc.
2+
3+
# This program is free software; you can redistribute it and/or modify
4+
# it under the terms of the GNU General Public License as published by
5+
# the Free Software Foundation; either version 3 of the License, or
6+
# (at your option) any later version.
7+
#
8+
# This program is distributed in the hope that it will be useful,
9+
# but WITHOUT ANY WARRANTY; without even the implied warranty of
10+
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
11+
# GNU General Public License for more details.
12+
#
13+
# You should have received a copy of the GNU General Public License
14+
# along with GCC; see the file COPYING3. If not see
15+
# <http://www.gnu.org/licenses/>.
16+
17+
# GCC testsuite that uses the `dg.exp' driver.
18+
19+
# Exit immediately if this isn't a m68k target.
20+
if ![istarget m68k*-*-*] then {
21+
return
22+
}
23+
24+
# Load support procs.
25+
load_lib g++-dg.exp
26+
27+
# Initialize `dg'.
28+
dg-init
29+
30+
# Main loop.
31+
dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/*.C]] "" ""
32+
33+
# All done.
34+
dg-finish

‎gcc/testsuite/g++.target/m68k/pr116244.C‎

Lines changed: 226 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,226 @@
1+
// { dg-do compile }
2+
// { dg-additional-options "-std=gnu++20 -O2 -w -march=isaa" }
3+
namespace std {
4+
struct __conditional {
5+
template <typename _Tp, typename> using type = _Tp;
6+
};
7+
template <bool, typename _If, typename _Else>
8+
using __conditional_t = __conditional::type<_If, _Else>;
9+
template <typename> constexpr bool is_lvalue_reference_v = true;
10+
template <typename> bool is_same_v;
11+
template <typename _From, typename _To>
12+
constexpr bool is_convertible_v = __is_convertible(_From, _To);
13+
template <typename> bool is_invocable_v;
14+
template <typename...> using common_reference_t = int;
15+
template <typename _Iterator> struct iterator_traits : _Iterator {};
16+
namespace __detail {
17+
template <typename, typename _Up>
18+
concept __same_as = is_same_v<_Up>;
19+
}
20+
template <typename _Tp, typename _Up>
21+
concept same_as = __detail::__same_as<_Up, _Tp>;
22+
template <typename _Derived, typename _Base>
23+
concept derived_from = is_convertible_v<_Derived, _Base>;
24+
template <typename, typename>
25+
concept common_reference_with =
26+
same_as<common_reference_t<>, common_reference_t<>>;
27+
namespace __detail {
28+
template <typename _Tp>
29+
concept __boolean_testable = requires(_Tp __t) { __t; };
30+
template <typename _Tp, typename>
31+
concept __weakly_eq_cmp_with = requires(_Tp __t) { __t; };
32+
} // namespace __detail
33+
template <typename... _Args>
34+
concept invocable = is_invocable_v<_Args...>;
35+
template <typename>
36+
concept regular_invocable = invocable<>;
37+
template <typename _Fn>
38+
concept predicate = __detail::__boolean_testable<_Fn>;
39+
template <typename _Rel, typename, typename>
40+
concept relation = predicate<_Rel>;
41+
template <typename _Rel, typename _Tp, typename _Up>
42+
concept strict_weak_order = relation<_Rel, _Tp, _Up>;
43+
namespace {
44+
struct duration {
45+
duration() = default;
46+
template <typename _Rep2> duration(_Rep2 __rep) : __r(__rep) {}
47+
long long count() { return __r; }
48+
long long __r;
49+
};
50+
long long operator+(duration __lhs, duration __rhs) {
51+
long __trans_tmp_1 = __lhs.count();
52+
return __trans_tmp_1 + __rhs.count();
53+
}
54+
struct time_point {
55+
time_point();
56+
time_point(duration __dur) : __d(__dur) {}
57+
duration time_since_epoch() { return __d; }
58+
duration __d;
59+
};
60+
time_point operator+(time_point __lhs, duration __rhs) {
61+
duration __trans_tmp_2 = __lhs.time_since_epoch();
62+
return time_point(__trans_tmp_2 + __rhs);
63+
}
64+
template <typename> using sys_time = time_point;
65+
} // namespace
66+
template <typename> class allocator;
67+
namespace {
68+
struct less {};
69+
} // namespace
70+
struct forward_iterator_tag {};
71+
namespace __detail {
72+
template <typename _Iter> struct __iter_traits_impl {
73+
using type = iterator_traits<_Iter>;
74+
};
75+
template <typename _Iter> using __iter_traits = __iter_traits_impl<_Iter>::type;
76+
template <typename> struct __iter_concept_impl;
77+
template <typename _Iter>
78+
requires requires { typename _Iter; }
79+
struct __iter_concept_impl<_Iter> {
80+
using type = __iter_traits<_Iter>::iterator_concept;
81+
};
82+
template <typename _Iter>
83+
using __iter_concept = __iter_concept_impl<_Iter>::type;
84+
template <typename _In>
85+
concept __indirectly_readable_impl = common_reference_with<_In, _In>;
86+
} // namespace __detail
87+
template <typename _In>
88+
concept indirectly_readable = __detail::__indirectly_readable_impl<_In>;
89+
template <typename _Iter>
90+
concept input_or_output_iterator = requires(_Iter __i) { __i; };
91+
template <typename _Sent, typename _Iter>
92+
concept sentinel_for = __detail::__weakly_eq_cmp_with<_Sent, _Iter>;
93+
template <typename _Iter>
94+
concept forward_iterator =
95+
derived_from<__detail::__iter_concept<_Iter>, forward_iterator_tag>;
96+
template <typename _Fn, typename>
97+
concept indirectly_regular_unary_invocable = regular_invocable<_Fn>;
98+
template <typename _Fn, typename _I1, typename _I2>
99+
concept indirect_strict_weak_order = strict_weak_order<_Fn, _I1, _I2>;
100+
namespace __detail {
101+
template <typename, typename> struct __projected;
102+
}
103+
template <indirectly_readable _Iter,
104+
indirectly_regular_unary_invocable<_Iter> _Proj>
105+
using projected = __detail::__projected<_Iter, _Proj>;
106+
namespace ranges::__access {
107+
template <typename _Tp> auto __begin(_Tp __t) { return __t.begin(); }
108+
} // namespace ranges::__access
109+
namespace __detail {
110+
template <typename _Tp>
111+
using __range_iter_t = decltype(ranges::__access::__begin(_Tp()));
112+
}
113+
template <typename _Tp> struct iterator_traits<_Tp *> {
114+
typedef forward_iterator_tag iterator_concept;
115+
using reference = _Tp;
116+
};
117+
} // namespace std
118+
template <typename _Iterator, typename> struct __normal_iterator {
119+
using iterator_concept = std::__detail::__iter_concept<_Iterator>;
120+
std::iterator_traits<_Iterator>::reference operator*();
121+
void operator++();
122+
};
123+
template <typename _IteratorL, typename _IteratorR, typename _Container>
124+
bool operator==(__normal_iterator<_IteratorL, _Container>,
125+
__normal_iterator<_IteratorR, _Container>);
126+
int _M_get_sys_info_ri;
127+
namespace std {
128+
template <typename> struct allocator_traits;
129+
template <typename _Tp> struct allocator_traits<allocator<_Tp>> {
130+
using pointer = _Tp *;
131+
};
132+
namespace {
133+
namespace __detail {
134+
template <typename _Tp>
135+
concept __maybe_borrowed_range = is_lvalue_reference_v<_Tp>;
136+
}
137+
int end;
138+
template <typename>
139+
concept range = requires { end; };
140+
template <typename _Tp>
141+
concept borrowed_range = __detail::__maybe_borrowed_range<_Tp>;
142+
template <typename _Tp> using iterator_t = std::__detail::__range_iter_t<_Tp>;
143+
template <typename _Tp>
144+
concept forward_range = forward_iterator<iterator_t<_Tp>>;
145+
struct dangling;
146+
template <input_or_output_iterator _It, sentinel_for<_It> _Sent = _It>
147+
struct subrange {
148+
_It begin();
149+
_Sent end();
150+
};
151+
template <range _Range>
152+
using borrowed_subrange_t =
153+
__conditional_t<borrowed_range<_Range>, subrange<iterator_t<_Range>>,
154+
dangling>;
155+
} // namespace
156+
template <typename _Alloc> struct _Vector_base {
157+
typedef allocator_traits<_Alloc>::pointer pointer;
158+
};
159+
template <typename _Tp, typename _Alloc = allocator<_Tp>> struct vector {
160+
__normal_iterator<typename _Vector_base<_Alloc>::pointer, int> begin();
161+
};
162+
template <typename _Tp> struct __shared_ptr_access {
163+
_Tp *operator->();
164+
};
165+
namespace chrono {
166+
struct weekday {
167+
char _M_wd;
168+
weekday _S_from_days() {
169+
long __trans_tmp_3;
170+
return 7 > __trans_tmp_3;
171+
}
172+
weekday(unsigned __wd) : _M_wd(__wd == 7 ?: __wd) {}
173+
weekday() : weekday{_S_from_days()} {}
174+
unsigned c_encoding() { return _M_wd; }
175+
friend duration operator-(weekday __x, weekday __y) {
176+
auto __n = __x.c_encoding() - __y.c_encoding();
177+
return int(__n) >= 0 ? __n : duration{};
178+
}
179+
};
180+
struct year_month_day {
181+
year_month_day _S_from_days(duration __dp) {
182+
auto __r0 = int(__dp.count()), __u2 = 5 * __r0;
183+
year_month_day{__u2};
184+
}
185+
year_month_day();
186+
year_month_day(int);
187+
year_month_day(sys_time<long> __dp)
188+
: year_month_day(_S_from_days(__dp.time_since_epoch())) {}
189+
};
190+
struct time_zone {
191+
void _M_get_sys_info() const;
192+
};
193+
} // namespace chrono
194+
namespace ranges {
195+
struct {
196+
template <
197+
forward_range _Range, typename _Tp, typename _Proj,
198+
indirect_strict_weak_order<_Tp, projected<_Range, _Proj>> _Comp = less>
199+
borrowed_subrange_t<_Range> operator()(_Range, _Tp, _Comp, _Proj);
200+
} equal_range;
201+
} // namespace ranges
202+
} // namespace std
203+
namespace std::chrono {
204+
struct Rule;
205+
unsigned day_of_week;
206+
struct _Node {
207+
vector<Rule> rules;
208+
};
209+
char name;
210+
struct Rule {
211+
int from;
212+
void start_time(int, long) {
213+
year_month_day ymd;
214+
sys_time<long> date;
215+
duration diff = day_of_week - weekday{};
216+
ymd = date + diff;
217+
}
218+
};
219+
__shared_ptr_access<_Node> __trans_tmp_5;
220+
void time_zone::_M_get_sys_info() const {
221+
auto node = __trans_tmp_5;
222+
auto rules = ranges::equal_range(node->rules, _M_get_sys_info_ri, {}, name);
223+
for (auto rule : rules)
224+
rule.start_time(rule.from, {});
225+
}
226+
} // namespace std::chrono

0 commit comments

Comments
 (0)