Re: [PATCH v3 1/3] arc: Add ARCv2 XML target along with refactoring

Simon Marchi
2020-07-22T13:49:52Z

On 2020-07-22 9:36 a.m., Shahab Vahedi wrote:
> On Thu, Jul 16, 2020 at 09:28:50AM -0400, Simon Marchi wrote:
>> On 2020-07-15 4:35 p.m., Shahab Vahedi wrote:
>> When I search for `arc_gdbarch_features_init` in that patch I don't find anything...
>
> You're correct. I have it in my local branch. The patch that uses it is going
> to be submitted very soon. I hope you don't mind if it remains like this.

Let's just make it static in this patch, and make in non-static again in your future
patch, it's not a big change.

>> The code in GDB constructing an arc_gdbarch_features will use the BFD to determine
>> these two values, whereas the code in GDBserver will use something else (usually
>> looking at the current process' properties).
>>
>> In fact, the constructor is optional, you could just build a arc_gdbarch_features
>> using aggregate initialization and return it from that function:
>>
>> arc_gdbarch_features features {reg_size, isa};
>>
>> It doesn't really matter. I just happen to prefer the constructor method, because
>> that makes it so you can't "forget" a field and it ensures it can never be in an
>> uninitialized state.
>
> I see now. Actually, I have usecases to not initialize it immediately,
> but in a few lines of code.

I'd be curious to see. Because instead of declaring it immediately, you can keep
the fields separate until you initialize it:

int reg_size;
enum arc_isa isa;

if (something)
{
reg_size = 8;
isa = foo;
}
else
{
reg_size = 4;
isa = bar;
}

arc_gdbarch_features features (reg_size, isa);

I think this is good, because the day you add a third axis to arc_gdbarch_features,
that code will not build and you'll be forced to updated it (you can't forget it).

Whereas with:

arc_gdbarch_features features;

if (something)
{
features.reg_size = 8;
features.isa = foo;
}
else
{
features.reg_size = 4;
features.isa = bar;
}

It's possible to forget.

But maybe you have a different use case in mind?

Simon