diff mbox series

Add rvalue::get_name method (and its C equivalent)

Message ID CAAOQCfSYhUkt57q1DLyQkbNROOVi3GJbEFsamcY8NdqvsTw2wg@mail.gmail.com
State New
Headers
Series Add rvalue::get_name method (and its C equivalent) |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gcc_check--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 success Testing passed

Commit Message

Guillaume Gomez April 19, 2024, 11:05 p.m. UTC 
  Hi,

I just encountered the need to retrieve the name of an `rvalue` (if
there is one) while working on the Rust GCC backend.

This patch adds a getter to retrieve the information.

Cordially.

Comments

Arthur Cohen April 22, 2024, 9:51 a.m. UTC | #1 
Hey Guillaume :)

On 4/20/24 01:05, Guillaume Gomez wrote:
> Hi,
> 
> I just encountered the need to retrieve the name of an `rvalue` (if
> there is one) while working on the Rust GCC backend.
> 
> This patch adds a getter to retrieve the information.
> 
> Cordially.

>    virtual bool get_wide_int (wide_int *) const { return false; }
>  
> +  virtual string * get_name () { return NULL; }
> +
>  private:
>    virtual enum precedence get_precedence () const = 0;

Is there any reason for that getter to return a mutable pointer to the 
name? Would something like this work instead if you're just looking at 
getting the name?

+  virtual string * get_name () const { return NULL; }

With of course adequate modifications to the inheriting classes.

Best,

Arthur
Guillaume Gomez April 22, 2024, 1:04 p.m. UTC | #2 
Hey Arthur :)

> Is there any reason for that getter to return a mutable pointer to the
> name? Would something like this work instead if you're just looking at
> getting the name?
>
> +  virtual string * get_name () const { return NULL; }
>
> With of course adequate modifications to the inheriting classes.

Good catch, thanks!

Updated the patch and attached the new version to this email.

Cordially.

Le lun. 22 avr. 2024 à 11:51, Arthur Cohen <arthur.cohen@embecosm.com> a écrit :
>
> Hey Guillaume :)
>
> On 4/20/24 01:05, Guillaume Gomez wrote:
> > Hi,
> >
> > I just encountered the need to retrieve the name of an `rvalue` (if
> > there is one) while working on the Rust GCC backend.
> >
> > This patch adds a getter to retrieve the information.
> >
> > Cordially.
>
> >    virtual bool get_wide_int (wide_int *) const { return false; }
> >
> > +  virtual string * get_name () { return NULL; }
> > +
> >  private:
> >    virtual enum precedence get_precedence () const = 0;
>
> Is there any reason for that getter to return a mutable pointer to the
> name? Would something like this work instead if you're just looking at
> getting the name?
>
> +  virtual string * get_name () const { return NULL; }
>
> With of course adequate modifications to the inheriting classes.
>
> Best,
>
> Arthur
Antoni Boucher April 22, 2024, 1:13 p.m. UTC | #3 
Please move the function to be on lvalue since there are no rvalue types 
that are not lvalues that have a name.

Le 2024-04-22 à 09 h 04, Guillaume Gomez a écrit :
> Hey Arthur :)
> 
>> Is there any reason for that getter to return a mutable pointer to the
>> name? Would something like this work instead if you're just looking at
>> getting the name?
>>
>> +  virtual string * get_name () const { return NULL; }
>>
>> With of course adequate modifications to the inheriting classes.
> 
> Good catch, thanks!
> 
> Updated the patch and attached the new version to this email.
> 
> Cordially.
> 
> Le lun. 22 avr. 2024 à 11:51, Arthur Cohen <arthur.cohen@embecosm.com> a écrit :
>>
>> Hey Guillaume :)
>>
>> On 4/20/24 01:05, Guillaume Gomez wrote:
>>> Hi,
>>>
>>> I just encountered the need to retrieve the name of an `rvalue` (if
>>> there is one) while working on the Rust GCC backend.
>>>
>>> This patch adds a getter to retrieve the information.
>>>
>>> Cordially.
>>
>>>     virtual bool get_wide_int (wide_int *) const { return false; }
>>>
>>> +  virtual string * get_name () { return NULL; }
>>> +
>>>   private:
>>>     virtual enum precedence get_precedence () const = 0;
>>
>> Is there any reason for that getter to return a mutable pointer to the
>> name? Would something like this work instead if you're just looking at
>> getting the name?
>>
>> +  virtual string * get_name () const { return NULL; }
>>
>> With of course adequate modifications to the inheriting classes.
>>
>> Best,
>>
>> Arthur
Guillaume Gomez April 22, 2024, 1:16 p.m. UTC | #4 
Good point!

New patch attached.

Le lun. 22 avr. 2024 à 15:13, Antoni Boucher <bouanto@zoho.com> a écrit :
>
> Please move the function to be on lvalue since there are no rvalue types
> that are not lvalues that have a name.
>
> Le 2024-04-22 à 09 h 04, Guillaume Gomez a écrit :
> > Hey Arthur :)
> >
> >> Is there any reason for that getter to return a mutable pointer to the
> >> name? Would something like this work instead if you're just looking at
> >> getting the name?
> >>
> >> +  virtual string * get_name () const { return NULL; }
> >>
> >> With of course adequate modifications to the inheriting classes.
> >
> > Good catch, thanks!
> >
> > Updated the patch and attached the new version to this email.
> >
> > Cordially.
> >
> > Le lun. 22 avr. 2024 à 11:51, Arthur Cohen <arthur.cohen@embecosm.com> a écrit :
> >>
> >> Hey Guillaume :)
> >>
> >> On 4/20/24 01:05, Guillaume Gomez wrote:
> >>> Hi,
> >>>
> >>> I just encountered the need to retrieve the name of an `rvalue` (if
> >>> there is one) while working on the Rust GCC backend.
> >>>
> >>> This patch adds a getter to retrieve the information.
> >>>
> >>> Cordially.
> >>
> >>>     virtual bool get_wide_int (wide_int *) const { return false; }
> >>>
> >>> +  virtual string * get_name () { return NULL; }
> >>> +
> >>>   private:
> >>>     virtual enum precedence get_precedence () const = 0;
> >>
> >> Is there any reason for that getter to return a mutable pointer to the
> >> name? Would something like this work instead if you're just looking at
> >> getting the name?
> >>
> >> +  virtual string * get_name () const { return NULL; }
> >>
> >> With of course adequate modifications to the inheriting classes.
> >>
> >> Best,
> >>
> >> Arthur
Antoni Boucher April 22, 2024, 1:19 p.m. UTC | #5 
I believe you forgot to regenerate the ChangeLog.

Le 2024-04-22 à 09 h 16, Guillaume Gomez a écrit :
> Good point!
> 
> New patch attached.
> 
> Le lun. 22 avr. 2024 à 15:13, Antoni Boucher <bouanto@zoho.com> a écrit :
>>
>> Please move the function to be on lvalue since there are no rvalue types
>> that are not lvalues that have a name.
>>
>> Le 2024-04-22 à 09 h 04, Guillaume Gomez a écrit :
>>> Hey Arthur :)
>>>
>>>> Is there any reason for that getter to return a mutable pointer to the
>>>> name? Would something like this work instead if you're just looking at
>>>> getting the name?
>>>>
>>>> +  virtual string * get_name () const { return NULL; }
>>>>
>>>> With of course adequate modifications to the inheriting classes.
>>>
>>> Good catch, thanks!
>>>
>>> Updated the patch and attached the new version to this email.
>>>
>>> Cordially.
>>>
>>> Le lun. 22 avr. 2024 à 11:51, Arthur Cohen <arthur.cohen@embecosm.com> a écrit :
>>>>
>>>> Hey Guillaume :)
>>>>
>>>> On 4/20/24 01:05, Guillaume Gomez wrote:
>>>>> Hi,
>>>>>
>>>>> I just encountered the need to retrieve the name of an `rvalue` (if
>>>>> there is one) while working on the Rust GCC backend.
>>>>>
>>>>> This patch adds a getter to retrieve the information.
>>>>>
>>>>> Cordially.
>>>>
>>>>>      virtual bool get_wide_int (wide_int *) const { return false; }
>>>>>
>>>>> +  virtual string * get_name () { return NULL; }
>>>>> +
>>>>>    private:
>>>>>      virtual enum precedence get_precedence () const = 0;
>>>>
>>>> Is there any reason for that getter to return a mutable pointer to the
>>>> name? Would something like this work instead if you're just looking at
>>>> getting the name?
>>>>
>>>> +  virtual string * get_name () const { return NULL; }
>>>>
>>>> With of course adequate modifications to the inheriting classes.
>>>>
>>>> Best,
>>>>
>>>> Arthur
Guillaume Gomez April 22, 2024, 1:24 p.m. UTC | #6 
... I most definitely forgot. Thanks! ^^'

Le lun. 22 avr. 2024 à 15:19, Antoni Boucher <bouanto@zoho.com> a écrit :

> I believe you forgot to regenerate the ChangeLog.
>
> Le 2024-04-22 à 09 h 16, Guillaume Gomez a écrit :
> > Good point!
> >
> > New patch attached.
> >
> > Le lun. 22 avr. 2024 à 15:13, Antoni Boucher <bouanto@zoho.com> a écrit
> :
> >>
> >> Please move the function to be on lvalue since there are no rvalue types
> >> that are not lvalues that have a name.
> >>
> >> Le 2024-04-22 à 09 h 04, Guillaume Gomez a écrit :
> >>> Hey Arthur :)
> >>>
> >>>> Is there any reason for that getter to return a mutable pointer to the
> >>>> name? Would something like this work instead if you're just looking at
> >>>> getting the name?
> >>>>
> >>>> +  virtual string * get_name () const { return NULL; }
> >>>>
> >>>> With of course adequate modifications to the inheriting classes.
> >>>
> >>> Good catch, thanks!
> >>>
> >>> Updated the patch and attached the new version to this email.
> >>>
> >>> Cordially.
> >>>
> >>> Le lun. 22 avr. 2024 à 11:51, Arthur Cohen <arthur.cohen@embecosm.com>
> a écrit :
> >>>>
> >>>> Hey Guillaume :)
> >>>>
> >>>> On 4/20/24 01:05, Guillaume Gomez wrote:
> >>>>> Hi,
> >>>>>
> >>>>> I just encountered the need to retrieve the name of an `rvalue` (if
> >>>>> there is one) while working on the Rust GCC backend.
> >>>>>
> >>>>> This patch adds a getter to retrieve the information.
> >>>>>
> >>>>> Cordially.
> >>>>
> >>>>>      virtual bool get_wide_int (wide_int *) const { return false; }
> >>>>>
> >>>>> +  virtual string * get_name () { return NULL; }
> >>>>> +
> >>>>>    private:
> >>>>>      virtual enum precedence get_precedence () const = 0;
> >>>>
> >>>> Is there any reason for that getter to return a mutable pointer to the
> >>>> name? Would something like this work instead if you're just looking at
> >>>> getting the name?
> >>>>
> >>>> +  virtual string * get_name () const { return NULL; }
> >>>>
> >>>> With of course adequate modifications to the inheriting classes.
> >>>>
> >>>> Best,
> >>>>
> >>>> Arthur
>
Antoni Boucher April 22, 2024, 3:19 p.m. UTC | #7 
For your new API endpoint, please add a check like:

   RETURN_IF_FAIL (lvalue->is_global () || lvalue->is_local (),
		  NULL,
		  NULL,
		  "lvalue should be a variable");


Le 2024-04-22 à 09 h 16, Guillaume Gomez a écrit :
> Good point!
> 
> New patch attached.
> 
> Le lun. 22 avr. 2024 à 15:13, Antoni Boucher <bouanto@zoho.com> a écrit :
>>
>> Please move the function to be on lvalue since there are no rvalue types
>> that are not lvalues that have a name.
>>
>> Le 2024-04-22 à 09 h 04, Guillaume Gomez a écrit :
>>> Hey Arthur :)
>>>
>>>> Is there any reason for that getter to return a mutable pointer to the
>>>> name? Would something like this work instead if you're just looking at
>>>> getting the name?
>>>>
>>>> +  virtual string * get_name () const { return NULL; }
>>>>
>>>> With of course adequate modifications to the inheriting classes.
>>>
>>> Good catch, thanks!
>>>
>>> Updated the patch and attached the new version to this email.
>>>
>>> Cordially.
>>>
>>> Le lun. 22 avr. 2024 à 11:51, Arthur Cohen <arthur.cohen@embecosm.com> a écrit :
>>>>
>>>> Hey Guillaume :)
>>>>
>>>> On 4/20/24 01:05, Guillaume Gomez wrote:
>>>>> Hi,
>>>>>
>>>>> I just encountered the need to retrieve the name of an `rvalue` (if
>>>>> there is one) while working on the Rust GCC backend.
>>>>>
>>>>> This patch adds a getter to retrieve the information.
>>>>>
>>>>> Cordially.
>>>>
>>>>>      virtual bool get_wide_int (wide_int *) const { return false; }
>>>>>
>>>>> +  virtual string * get_name () { return NULL; }
>>>>> +
>>>>>    private:
>>>>>      virtual enum precedence get_precedence () const = 0;
>>>>
>>>> Is there any reason for that getter to return a mutable pointer to the
>>>> name? Would something like this work instead if you're just looking at
>>>> getting the name?
>>>>
>>>> +  virtual string * get_name () const { return NULL; }
>>>>
>>>> With of course adequate modifications to the inheriting classes.
>>>>
>>>> Best,
>>>>
>>>> Arthur
Guillaume Gomez April 22, 2024, 5:56 p.m. UTC | #8 
`param` is also inheriting from `lvalue`. I don't think adding this
check is a good idea
because it will not evolve nicely if more changes are done in libgccjit.

Le lun. 22 avr. 2024 à 17:19, Antoni Boucher <bouanto@zoho.com> a écrit :
>
> For your new API endpoint, please add a check like:
>
>    RETURN_IF_FAIL (lvalue->is_global () || lvalue->is_local (),
>                   NULL,
>                   NULL,
>                   "lvalue should be a variable");
>
>
> Le 2024-04-22 à 09 h 16, Guillaume Gomez a écrit :
> > Good point!
> >
> > New patch attached.
> >
> > Le lun. 22 avr. 2024 à 15:13, Antoni Boucher <bouanto@zoho.com> a écrit :
> >>
> >> Please move the function to be on lvalue since there are no rvalue types
> >> that are not lvalues that have a name.
> >>
> >> Le 2024-04-22 à 09 h 04, Guillaume Gomez a écrit :
> >>> Hey Arthur :)
> >>>
> >>>> Is there any reason for that getter to return a mutable pointer to the
> >>>> name? Would something like this work instead if you're just looking at
> >>>> getting the name?
> >>>>
> >>>> +  virtual string * get_name () const { return NULL; }
> >>>>
> >>>> With of course adequate modifications to the inheriting classes.
> >>>
> >>> Good catch, thanks!
> >>>
> >>> Updated the patch and attached the new version to this email.
> >>>
> >>> Cordially.
> >>>
> >>> Le lun. 22 avr. 2024 à 11:51, Arthur Cohen <arthur.cohen@embecosm.com> a écrit :
> >>>>
> >>>> Hey Guillaume :)
> >>>>
> >>>> On 4/20/24 01:05, Guillaume Gomez wrote:
> >>>>> Hi,
> >>>>>
> >>>>> I just encountered the need to retrieve the name of an `rvalue` (if
> >>>>> there is one) while working on the Rust GCC backend.
> >>>>>
> >>>>> This patch adds a getter to retrieve the information.
> >>>>>
> >>>>> Cordially.
> >>>>
> >>>>>      virtual bool get_wide_int (wide_int *) const { return false; }
> >>>>>
> >>>>> +  virtual string * get_name () { return NULL; }
> >>>>> +
> >>>>>    private:
> >>>>>      virtual enum precedence get_precedence () const = 0;
> >>>>
> >>>> Is there any reason for that getter to return a mutable pointer to the
> >>>> name? Would something like this work instead if you're just looking at
> >>>> getting the name?
> >>>>
> >>>> +  virtual string * get_name () const { return NULL; }
> >>>>
> >>>> With of course adequate modifications to the inheriting classes.
> >>>>
> >>>> Best,
> >>>>
> >>>> Arthur
diff mbox series

Patch

From d2ddeec950f23533e5e18bc0c10c4b49eef3cda3 Mon Sep 17 00:00:00 2001
From: Guillaume Gomez <guillaume1.gomez@gmail.com>
Date: Sat, 20 Apr 2024 01:02:20 +0200
Subject: [PATCH] [PATCH] Add rvalue::get_name method

gcc/jit/ChangeLog:

	* jit-recording.h: Add rvalue::get_name method
	* libgccjit.cc (gcc_jit_rvalue_get_name): Likewise
	* libgccjit.h (gcc_jit_rvalue_get_name): Likewise
	* libgccjit.map: Likewise

gcc/testsuite/ChangeLog:

	* jit.dg/test-tls.c: Add test for gcc_jit_rvalue_get_name
---
 gcc/jit/jit-recording.h         |  8 ++++++++
 gcc/jit/libgccjit.cc            | 16 ++++++++++++++++
 gcc/jit/libgccjit.h             |  4 ++++
 gcc/jit/libgccjit.map           |  5 +++++
 gcc/testsuite/jit.dg/test-tls.c |  3 +++
 5 files changed, 36 insertions(+)

diff --git a/gcc/jit/jit-recording.h b/gcc/jit/jit-recording.h
index d8d16f4fe29..3ae87c146ac 100644
--- a/gcc/jit/jit-recording.h
+++ b/gcc/jit/jit-recording.h
@@ -1213,6 +1213,8 @@  public:
   virtual bool is_constant () const { return false; }
   virtual bool get_wide_int (wide_int *) const { return false; }
 
+  virtual string * get_name () { return NULL; }
+
 private:
   virtual enum precedence get_precedence () const = 0;
 
@@ -1305,6 +1307,8 @@  public:
   const char *access_as_rvalue (reproducer &r) final override;
   const char *access_as_lvalue (reproducer &r) final override;
 
+  string * get_name () final override { return m_name; }
+
 private:
   string * make_debug_string () final override { return m_name; }
   void write_reproducer (reproducer &r) final override;
@@ -1558,6 +1562,8 @@  public:
 
   void set_rvalue_init (rvalue *val) { m_rvalue_init = val; }
 
+  string * get_name () final override { return m_name; }
+
 private:
   string * make_debug_string () final override { return m_name; }
   template <typename T>
@@ -2148,6 +2154,8 @@  public:
 
   void write_to_dump (dump &d) final override;
 
+  string * get_name () final override { return m_name; }
+
 private:
   string * make_debug_string () final override { return m_name; }
   void write_reproducer (reproducer &r) final override;
diff --git a/gcc/jit/libgccjit.cc b/gcc/jit/libgccjit.cc
index 445c0d0e0e3..2b8706dc7fd 100644
--- a/gcc/jit/libgccjit.cc
+++ b/gcc/jit/libgccjit.cc
@@ -4377,3 +4377,19 @@  gcc_jit_context_add_top_level_asm (gcc_jit_context *ctxt,
   RETURN_IF_FAIL (asm_stmts, ctxt, NULL, "NULL asm_stmts");
   ctxt->add_top_level_asm (loc, asm_stmts);
 }
+
+/* Public entrypoint.  See description in libgccjit.h.
+
+   After error-checking, this calls the trivial
+   gcc::jit::recording::rvalue::get_name method, in jit-recording.h.  */
+
+extern const char *
+gcc_jit_rvalue_get_name (gcc_jit_rvalue *rvalue)
+{
+  RETURN_NULL_IF_FAIL (rvalue, NULL, NULL, "NULL rvalue");
+  auto name = rvalue->get_name ();
+
+  if (!name)
+    return NULL;
+  return name->c_str ();
+}
diff --git a/gcc/jit/libgccjit.h b/gcc/jit/libgccjit.h
index 74e847b2dec..d4094610a16 100644
--- a/gcc/jit/libgccjit.h
+++ b/gcc/jit/libgccjit.h
@@ -2066,6 +2066,10 @@  gcc_jit_lvalue_add_string_attribute (gcc_jit_lvalue *variable,
 				     enum gcc_jit_variable_attribute attribute,
 				     const char* value);
 
+/* Returns the name of the `rvalue`, if any. Returns NULL otherwise.  */
+extern const char *
+gcc_jit_rvalue_get_name (gcc_jit_rvalue *rvalue);
+
 #ifdef __cplusplus
 }
 #endif /* __cplusplus */
diff --git a/gcc/jit/libgccjit.map b/gcc/jit/libgccjit.map
index 99aa5970be1..bbed8024263 100644
--- a/gcc/jit/libgccjit.map
+++ b/gcc/jit/libgccjit.map
@@ -289,3 +289,8 @@  LIBGCCJIT_ABI_27 {
   global:
     gcc_jit_context_new_sizeof;
 } LIBGCCJIT_ABI_26;
+
+LIBGCCJIT_ABI_28 {
+  global:
+    gcc_jit_rvalue_get_name;
+} LIBGCCJIT_ABI_27;
diff --git a/gcc/testsuite/jit.dg/test-tls.c b/gcc/testsuite/jit.dg/test-tls.c
index 3b20182ac10..b651eb09b44 100644
--- a/gcc/testsuite/jit.dg/test-tls.c
+++ b/gcc/testsuite/jit.dg/test-tls.c
@@ -28,6 +28,9 @@  create_code (gcc_jit_context *ctxt, void *user_data)
       ctxt, NULL, GCC_JIT_GLOBAL_EXPORTED, int_type, "foo");
   gcc_jit_lvalue_set_tls_model (foo, GCC_JIT_TLS_MODEL_GLOBAL_DYNAMIC);
 
+  CHECK_STRING_VALUE (
+    gcc_jit_rvalue_get_name (gcc_jit_lvalue_as_rvalue (foo)), "foo");
+
   /* Build the test_fn.  */
   gcc_jit_function *test_fn =
     gcc_jit_context_new_function (ctxt, NULL,
-- 
2.24.1.2762.gfe2e4819b8