From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 7E8B43858C3A for ; Sun, 21 Nov 2021 20:28:35 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 7E8B43858C3A Received: from mail-qk1-f200.google.com (mail-qk1-f200.google.com [209.85.222.200]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-510-RIUhUIDyMhaX4Nasf1St4w-1; Sun, 21 Nov 2021 15:28:32 -0500 X-MC-Unique: RIUhUIDyMhaX4Nasf1St4w-1 Received: by mail-qk1-f200.google.com with SMTP id u8-20020a05620a454800b00468482aac5dso13230132qkp.18 for ; Sun, 21 Nov 2021 12:28:31 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:subject:from:to:date:in-reply-to :references:user-agent:mime-version:content-transfer-encoding; bh=XSdT1ch/8VMNfNPGWQqWuKTOn2QkI+2J9AbP8ODDcXs=; b=2tsU8VxW16qAbayVvnk7s2JNkd/LDlEHGy3pBlIwrpogPXmRMAiXXGxsDEUGJAszFl DuLm1oGiuzkvrqZlFzGDBXsvGq7E3JujuIlF0JSf+p6AydjrHhMLo3se63bUodpxeBFL IGrBO/SVOvJGn8+IfpNnhjigAMN9VTGw7lBSrRIm+TJ6ijwJL9i90Vfat26H5tA3rSDM 0wFIkCH2jUz9RKHRG00LjH8h81b5W0rDU+0cq52a1gWsqLAvDF66IAclVge9SBD9wk/z 1q2M2+I7T5QVjJXxvYcmGpJgdq20qlEcQnscb4PmNDuHWq5n40fhgOu5bEAISDCU4nDm 5LnQ== X-Gm-Message-State: AOAM532N+PuRFOEuAM8odBXItX2pDtm59CZ1KIDBxhJfH7wlIZz2CCfO kUZzZveVievIs6DSQ2OJFXU9yTbie83HcafyYUNRvcSOMhMX9ZKABfoEcstFjMWfgntlKWzIDjs rc8KXH1c= X-Received: by 2002:a05:6214:2a45:: with SMTP id jf5mr94736289qvb.15.1637526511535; Sun, 21 Nov 2021 12:28:31 -0800 (PST) X-Google-Smtp-Source: ABdhPJwTt1XLR1nS8hW9rbW7Hni8Wcg1+bxW1QhPIx3dRiOKuDL1mn2Ai1BmWS/JLHoxX2P0pUAriw== X-Received: by 2002:a05:6214:2a45:: with SMTP id jf5mr94736267qvb.15.1637526511307; Sun, 21 Nov 2021 12:28:31 -0800 (PST) Received: from t14s.localdomain (c-73-69-212-193.hsd1.ma.comcast.net. [73.69.212.193]) by smtp.gmail.com with ESMTPSA id h13sm3394047qtk.25.2021.11.21.12.28.30 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 21 Nov 2021 12:28:30 -0800 (PST) Message-ID: <21d74ff9ab1a2645cd93a71a4e0343912020ffd2.camel@redhat.com> Subject: Re: [PATCH] libgccjit: Add support for TLS variable [PR95415] From: David Malcolm To: Antoni Boucher , jit@gcc.gnu.org, gcc-patches@gcc.gnu.org Date: Sun, 21 Nov 2021 15:28:29 -0500 In-Reply-To: References: <750892c90ccd75ae1df099b829ce2d51fe886195.camel@zoho.com> User-Agent: Evolution 3.38.4 (3.38.4-1.fc33) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-10.7 required=5.0 tests=BAYES_00, BODY_8BITS, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: jit@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Jit mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 21 Nov 2021 20:28:37 -0000 On Sat, 2021-11-20 at 17:34 -0500, Antoni Boucher wrote: > Hi. > Here's the updated patch. > See comments below. > Thanks for your reviews! > > Le jeudi 20 mai 2021 à 16:11 -0400, David Malcolm a écrit : > > On Tue, 2021-05-18 at 20:43 -0400, Antoni Boucher via Gcc-patches > > wrote: > > > Hello. > > > This patch adds support for TLS variables. > > > One thing to fix before we merge it is the libgccjit.map file > > > which > > > contains LIBGCCJIT_ABI_16 instead of LIBGCCJIT_ABI_17. > > > LIBGCCJIT_ABI_16 was added in one of my other patches. > > > Thanks for the review. > > > > > diff --git a/gcc/jit/docs/topics/compatibility.rst > > > b/gcc/jit/docs/topics/compatibility.rst > > > index 239b6aa1a92..d10bc1df080 100644 > > > --- a/gcc/jit/docs/topics/compatibility.rst > > > +++ b/gcc/jit/docs/topics/compatibility.rst > > > @@ -243,3 +243,12 @@ embedding assembler instructions: > > >    * :func:`gcc_jit_extended_asm_add_input_operand` > > >    * :func:`gcc_jit_extended_asm_add_clobber` > > >    * :func:`gcc_jit_context_add_top_level_asm` > > > + > > > +.. _LIBGCCJIT_ABI_17: > > > + > > > +``LIBGCCJIT_ABI_17`` > > > +----------------------- > > > +``LIBGCCJIT_ABI_17`` covers the addition of an API entrypoint to > > > set > > > the > > > +thread-local storage model of a variable: > > > + > > > +  * :func:`gcc_jit_lvalue_set_tls_model` > > > > Sorry about the delay in reviewing patches. > > > > Is there a summary somewhere of the various outstanding patches and > > their associated ABI versions?  Are there dependencies between the > > patches? > > The list of patches is there: > https://github.com/antoyo/libgccjit-patches but I don't keep them up- > to-date. > If that would help you, I could add a README to tell what is the new > ABI version for each patch. > I believe there might be some patches that depend on a previous one. That's not needed; I think all I need to know is what the next patch you need me to look at is (FWIW I'm about to go on vacation for a week) [...snip...] > > > > > > + > > > +void > > > +create_code (gcc_jit_context *ctxt, void *user_data) > > > +{ > > > +  /* Let's try to inject the equivalent of: > > > + > > > +     _Thread_local int foo; > > > +  */ > > > +  gcc_jit_type *int_type = > > > +    gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_INT); > > > + > > > +  gcc_jit_lvalue *foo = > > > +    gcc_jit_context_new_global ( > > > +      ctxt, NULL, GCC_JIT_GLOBAL_EXPORTED, int_type, "foo"); > > > +  gcc_jit_lvalue_set_tls_model (foo, > > > GCC_JIT_TLS_MODEL_GLOBAL_DYNAMIC); > > > > How many of the different enum values can be supported?  How > > target- > > dependent is this? > > I'm not sure what you mean here. Are you asking that I test all the > different enum values? That would be ideal, but I don't think it's necessary. > The tls_model enum is defined in gcc/coretypes.h and does not seem to > change depending on the target. Maybe there are checks elsewhere for > that, though. It might be that some targets only support some modes; I don't know. [...snip...] Thanks for the updated patch. It looks good to push to trunk once the earlier ones are in place, though as usual please re-test it before pushing. Dave