[PATCH v1 1/2][PPC64] [PR88877]

Segher Boessenkool segher@kernel.crashing.org
Sun May 24 16:22:31 GMT 2020


Hi!

On Sun, May 24, 2020 at 07:03:13PM +0530, Kamlesh Kumar wrote:
> In this patch series trying to address same by creating a struct Tuple
> which bundles existing rtx and machine_mode and added one more
> bool member which store unsigned_p which by default is false.

The idea is good.  However, you cannot call something as specific as this
"tuple", in a header file that is used everywhere even.  (We also do not
have a "leading caps on types" convention).

> This patch does not change underlying behavior yet. This will be done in
> follow up patches.

Thanks :-)

>         * rtl.h (Tuple): Defined and typedefed to rtx_mode_t.

It's the other way around: rtx_mode_t is typedeffed to struct Tuple, so
rtx_mode_t should be listed to the left of a : as well.

OTOH, you don't need to name Tuple at all...  It should not *have* a
constructor, since you declared it as class...  But you can just use
std::tuple here?

>         (emit_library_call): Added default arg unsigned_p.
>         (emit_library_call_value): Added default arg unsigned_p.

Yeah, eww.  Default arguments have all the problems you had before,
except now it is hidden and much more surprising.

Those functions really should take rtx_mode_t arguments?

Thanks again for working on this,


Segher


More information about the Gcc-patches mailing list