fsep is slow even when we don't benefit from it, relax its use?

[Ptival]

Creating as draft to have a discussion on design.

I noticed we pay a huge performance cost on running fsep, even though we use an unbounded width layout. I'm intending to ask upstream to pretty (also prettyprinter) whether they can do anything about this, but in the meantime, it'd be nice if llvm-pretty was flexible enough to allow opting out of fsep in favor of hsep when we know there is no reason to try and "fill a line".

This example PR works (and takes the pretty-printing down from ~30 seconds to ~3 seconds on my use case).

I stashed the configuration with the LLVM one, but this is completely unrelated. Should it warrant a second, separate implicit configuration? Or is there a nicer way of achieving this?

[elliottt]

What about always using hsep? I'm not sure it's very important to fit within the doc size, given that the pretty-printed IR is consumed by llvm. Are there many cases where readability is significantly improved by using fsep?

[Ptival]

It makes long type descriptions more readable, e.g.

source_filename = "llvm-link"
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
%class.AirshipAttitudeControl = type { %class.ModuleBase.base,
                                       %"class.uORB::SubscriptionCallback",
                                       %"class.px4::WorkItem",
                                       %"class.uORB::SubscriptionInterval",
                                       %"class.uORB::Subscription.18",
                                       %"class.uORB::Subscription.18",
                                       %"class.uORB::SubscriptionCallbackWorkItem",
                                       %"class.uORB::Publication",
                                       %struct.manual_control_setpoint_s,
                                       %struct.vehicle_status_s,
                                       %struct.actuator_controls_s,
                                       %struct.perf_ctr_header* }
%class.AirspeedModule = type { %class.ModuleBase.base,
                               %"class.uORB::SubscriptionCallback",
                               %"class.px4::ScheduledWorkItem",
                               %"class.uORB::Publication",
                               [4 x %"class.uORB::Publication"], i8*,
                               %"class.uORB::SubscriptionInterval",
                               %"class.uORB::Subscription.18",
                               %"class.uORB::Subscription.18",
                               %"class.uORB::Subscription.18",
                               %"class.uORB::Subscription.18",
                               %"class.uORB::Subscription.18",
                               %"class.uORB::Subscription.18",
                               %"class.uORB::Subscription.18",
                               %"class.uORB::Subscription.18",
                               %"class.uORB::Subscription.18",
                               %"class.uORB::Subscription.18",
                               %"class.uORB::SubscriptionMultiArray.3196",
                               %struct.estimator_status_s,
                               %struct.vehicle_angular_velocity_s,
                               %struct.vehicle_air_data_s,
                               %struct.vehicle_attitude_s,
                               %struct.vehicle_land_detected_s,
                               %struct.vehicle_local_position_s,
                               %struct.vehicle_status_s,
                               %struct.vtol_vehicle_status_s,
                               %struct.position_setpoint_s,
                               %class.WindEstimator, %struct.airspeed_wind_s,
                               i32, i32, [3 x %class.AirspeedValidator], i64,
                               i32, i32, i8, i8, i8, float, float, i8,
                               [3 x i64], %struct.perf_ctr_header*,
                               %"class.do_not_explicitly_use_this_namespace::Param",
                               %"class.do_not_explicitly_use_this_namespace::Param",
                               %"class.do_not_explicitly_use_this_namespace::Param",
                               %"class.do_not_explicitly_use_this_namespace::Param",
                               %"class.px4::atomic", %"class.px4::atomic",
                               %"class.px4::atomic",
                               %"class.do_not_explicitly_use_this_namespace::Param",
                               %"class.px4::atomic", %"class.px4::atomic",
                               %"class.px4::atomic",
                               %"class.do_not_explicitly_use_this_namespace::Param",
                               %"class.do_not_explicitly_use_this_namespace::Param",
                               %"class.px4::atomic", %"class.px4::atomic",
                               %"class.do_not_explicitly_use_this_namespace::Param" }
%class.AirspeedValidator = type { %class.WindEstimator, i8, float, i8, float,
                                  float, float, float, float, i8, float, float,
                                  i64, i64, float, i64, i8, float, float, i8,
                                  i32, i32, i64, i64 }

But indeed I don't know whether this is ever helpful.

I had attempted to preserve this behavior just in case someone was relying on it, but if you think it's unlikely, I think it'd be reasonable to just switch to hsep until someone complains they actually want to pretty-print.

[elliottt]

At this point I think that Galois is the heaviest user of llvm-pretty, so I'd say it's somewhat up to you. Do you end up debugging llvm ir that has a lot of long structs in it?

Another alternative would be to look at changing the underlying pretty-printing library. I think that wl-pprint might perform better here as the line wrapping logic is a bit less complicated?