RFR: 8325674: Constant fold across compares [v2]
Emanuel Peter
epeter at openjdk.org
Tue Feb 27 09:54:07 UTC 2024
On Mon, 26 Feb 2024 23:20:57 GMT, Joshua Cao <duke at openjdk.org> wrote:
>> src/hotspot/share/opto/subnode.cpp line 1580:
>>
>>> 1578: _test._test == BoolTest::eq || _test._test == BoolTest::ne) {
>>> 1579: cmp = CmpNode::make(phase->transform(SubNode::make(c1, cmp2, bt)), x,
>>> 1580: bt);
>>
>> Suggestion:
>>
>> cmp = CmpNode::make(phase->transform(SubNode::make(c1, cmp2, bt)), x, bt);
>
> I added these suggestions. Is there any guidelines for number of columns? My editor auto-formats it to 80 columns following [clangd's defaults](https://llvm.org/docs/CodingStandards.html#source-code-width).
> There is no hard line length limit. That said, bear in mind that excessively long lines can cause difficulties. Some people like to have multiple side-by-side windows in their editors, and long lines may force them to choose among unpleasant options. They can use wide windows, reducing the number that can fit across the screen, and wasting a lot of screen real estate because most lines are not that long. Alternatively, they can have more windows across the screen, with long lines wrapping (or worse, requiring scrolling to see in their entirety), which is harder to read. Similar issues exist for side-by-side code reviews.
That is all I can find. I think `80` is a bit strict, but people will probably argue about that.
In this case, it was simply a few characters more than `80`, and taking them onto the next line seems "noisy", i.e. makes it harder to read.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/17853#discussion_r1503947609
More information about the hotspot-compiler-dev
mailing list