LLVM Bugzilla is read-only and represents the historical archive of all LLVM issues filled before November 26, 2021. Use github to submit LLVM bugs

Bug 51211 - [SimplifyCFG] Chain of comparisons no longer flattened
Summary: [SimplifyCFG] Chain of comparisons no longer flattened
Status: NEW
Alias: None
Product: libraries
Classification: Unclassified
Component: Scalar Optimizations (show other bugs)
Version: trunk
Hardware: PC Linux
: P enhancement
Assignee: Unassigned LLVM Bugs
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-07-25 13:25 PDT by Nikita Popov
Modified: 2021-07-27 13:54 PDT (History)
6 users (show)

See Also:
Fixed By Commit(s):


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Nikita Popov 2021-07-25 13:25:25 PDT
For the following Rust code:

pub struct Blueprint {
    pub fuel_tank_size: u32,
    pub payload: u32,
    pub wheel_diameter: u32,
    pub wheel_width: u32,
}
impl PartialEq for Blueprint{
    fn eq(&self, other: &Self) -> bool {
       self.fuel_tank_size == other.fuel_tank_size
            && self.payload == other.payload
            && self.wheel_diameter == other.wheel_diameter
            && self.wheel_width == other.wheel_width
    }
}

LLVM 12 managed to compile this to a nice <4 x i32> comparison, while in LLVM 13 this is a sequence of i32 comparisons with two branches sitting in between.

Here is an end-to-end IR test case: https://llvm.godbolt.org/z/59cT1coT6

The relevant difference is that SimplifyCFG no longer fully flattens the control flow in this case.

Here is just the -simplifycfg part: https://llvm.godbolt.org/z/zfd9Ybx58

I haven't checked what change caused this yet.
Comment 1 Nikita Popov 2021-07-25 13:57:56 PDT
Interestingly, doing something similar in clang doesn't flatten at all, on any version: https://c.godbolt.org/z/aTWWxTx8h

Which makes me suspect that this kind of aggressive flattening was not actually supposed to happen, and we just got "lucky" with transform ordering in SimplifyCFG, or similar.
Comment 2 Roman Lebedev 2021-07-25 14:12:30 PDT
I can't imagine that such a flattening in itself would be good.
I agree that *if* it vectorizes, then it is beneficial.
But looking just at the final ir after simplifycfg,
i'm not quite seeing how we can flatten it within the costmodel.
Comment 4 Nikita Popov 2021-07-26 01:06:34 PDT
It looks like MergeICmps does handle this in the clang case, but not in the rustc case -- apparently the fact that the first two conditions are flattened makes MergeICmps no longer recognize the pattern. So there's possibly two bugs here:

1. MergeICmps should also recognize conditions combined with &&, not just branches.
2. SimplfyCFG should not be flattening the first two conditions, as it doesn't do so if the starting CFG structure is different.
Comment 5 Nikita Popov 2021-07-27 13:54:29 PDT
Worth noting that https://reviews.llvm.org/D106593 would bring this back to flattening everything -- which is probably an argument against D106593?