From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mo4-p00-ob.smtp.rzone.de (mo4-p00-ob.smtp.rzone.de [85.215.255.20]) by sourceware.org (Postfix) with ESMTPS id 44DCD3893662 for ; Tue, 15 Nov 2022 12:15:07 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 44DCD3893662 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=gjlay.de Authentication-Results: sourceware.org; spf=none smtp.mailfrom=gjlay.de DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; t=1668514505; s=strato-dkim-0002; d=gjlay.de; h=In-Reply-To:From:References:Cc:To:Subject:Date:Message-ID:Cc:Date: From:Subject:Sender; bh=Zn73OMDagcAEc5/OE3R/NefEw0Jtp+h6N5RsYAjYKqE=; b=GxdcQ47fO4c5wVj6lLwH4FB5tmsMEQmjT7clUfcaD4mSQ5DTLzutCPM8kWlixLqJlp jNh4DVmgWe7Z0vYLABF+JppewLyiW1FV40lwvVpiAnEuecJw3HF0O6fmq4L0VWtCaSez lJMQ70AVGVZ6Z2WAbqB2L01lEdSeqp8cQ7ELiP5nmamdLa6BcnNER7R1UKK4nVcrnlfB snmox39Lp0+S1cX+Gu0XTtPK1HZpYWEFNiuPVkFnUMmEVLCOKtup1PXLEMFleOjI10ra OXKlFT5E5PungWL63JFNYmU6Ipvp73cvidZM4nV2v41pWilUega902FpcX9cS+j9OJaP ThaA== Authentication-Results: strato.com; dkim=none X-RZG-AUTH: ":LXoWVUeid/7A29J/hMvvT3koxZnKT7Qq0xotTetVnKkbjtK7qmy9Jvpc5Ezo" X-RZG-CLASS-ID: mo00 Received: from [192.168.2.102] by smtp.strato.de (RZmta 48.2.1 DYNA|AUTH) with ESMTPSA id aba1dcyAFCF5qZ6 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256 bits)) (Client did not present a certificate); Tue, 15 Nov 2022 13:15:05 +0100 (CET) Message-ID: <30df2a2b-c895-1ae6-100a-ad1450a5ba22@gjlay.de> Date: Tue, 15 Nov 2022 13:15:04 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.2.2 Subject: Re: [PATCH v3] c++: parser - Support for target address spaces in C++ Content-Language: en-US To: Jason Merrill , Paul Iannetta Cc: gcc-patches@gcc.gnu.org References: <20221013160227.sdlv6yaw5gr4zcvd@ws2202.lin.mbt.kalray.eu> <20221013215643.o2bymrmffwbtuppu@ws2202.lin.mbt.kalray.eu> <4026cae9-e371-a2ee-2b36-7abc9224afa1@redhat.com> <20221018073731.wj2expjfmk5uhmp3@ws2202.lin.mbt.kalray.eu> <07d4c9ba-594a-d3f8-3df3-5ef5d18a6e97@redhat.com> <20221018170135.zpkmyebmpcvqx7ky@ws2202.lin.mbt.kalray.eu> <20221026071837.l3de5hkxujvqqztr@ws2202.lin.mbt.kalray.eu> <49b9d82e-3252-9e58-f064-b74f059d8b7d@gjlay.de> <20221110140838.7uupbxeretumhugk@ws2202.lin.mbt.kalray.eu> <3803548f-9437-4939-4f35-cbbfb61fdc1b@gjlay.de> <2184e167-795f-126b-b535-fe3130441e08@redhat.com> From: Georg-Johann Lay In-Reply-To: <2184e167-795f-126b-b535-fe3130441e08@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-0.9 required=5.0 tests=BAYES_00,BODY_8BITS,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,KAM_ASCII_DIVIDERS,KAM_SHORT,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,SPF_HELO_PASS,SPF_NONE,TXREP autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Am 14.11.22 um 18:55 schrieb Jason Merrill: > On 11/10/22 06:40, Georg-Johann Lay wrote: >> >> >> Am 10.11.22 um 15:08 schrieb Paul Iannetta: >>> On Thu, Nov 03, 2022 at 02:38:39PM +0100, Georg-Johann Lay wrote: >>>>> [PATCH v3] c++: parser - Support for target address spaces in C++ >>>> 2. Will it work with compound literals? >>>> ======================================= >>>> >>>> Currently, the following C code works for target avr: >>>> >>>> const __flash char *pHallo = (const __flash char[]) { "Hallo" }; >>>> >>>> This is a pointer in RAM (AS0) that holds the address of a string in >>>> flash >>>> (AS1) and is initialized with that address. Unfortunately, this does >>>> not >>>> work locally: >>>> >>>> const __flash char* get_hallo (void) >>>> { >>>>      [static] const __flash char *p2 = (const __flash char[]) { >>>> "Hallo2" }; >>>>      return p2; >>>> } >>>> >>>> foo.c: In function 'get_hallo': >>>> foo.c: error: compound literal qualified by address-space qualifier >>>> >>>> Is there any way to make this work now? Would be great! > > I don't object to allowing this, but what's the advantage of this > pattern over > > static __flash const char p2[] = "Hallo2"; > > ? Hi Jason. Take an example that's a bit more complicated, like: #define FSTR(X) (const __flash char[]) { X } const __flash char* strings[] = { FSTR("cat"), FSTR("dog"), FSTR("petrophaga lorioti") }; This won't work in a function just because GCC rejects it, no matter whether strings[] itself is in __flash or not. One work around would be to const __flash char str_cat[] = "cat"; const __flash char str_dog[] = "dog"; const __flash char str_petrophaga_lorioti[] = "petrophaga_lorioti"; const __flash char* strings[] = { str_cat, str_dog, str_petrophaga_lorioti }; but anyone would prefer the first alternative. In a more broader context, code without ASes like const char* strings[] = { "cat", "dog", "petrophaga lorioti" }; that makes perfect sense in C/C++ should also be possible for address-spaces, no matter whether the pointers and/or strings[] is in non-generic AS. Unfortunately, ISO/IEC TR 18037 seems to never have considered such code, and they mostly are concerned with how code is being accessed, but not how code is being located. Johann >>> Currently, I implement the same restrictions as the C front-end, but I >>> think that this restriction could be lifted. >>>> 3. Will TARGET_ADDR_SPACE_DIAGNOSE_USAGE still work? >>>> ==================================================== >>>> >>>> Currently there is target hook TARGET_ADDR_SPACE_DIAGNOSE_USAGE. >>>> I did not see it in your patches, so maybe I just missed it? See >>>> https://gcc.gnu.org/onlinedocs/gcc-12.2.0/gccint/Named-Address-Spaces.html#index-TARGET_005fADDR_005fSPACE_005fDIAGNOSE_005fUSAGE >>> >>> That was a point I overlooked in my previous patch.  This will be in >>> my new revision where I also add implicit conversion between >>> address spaces and also expose TARGET_ADDR_SPACE_CONVERT. >>> >>>> 4. Will it be possible to put C++ virtual tables in ASs, and how? >>>> ================================================================= >>> >>> Currently, I do not allow the declaration of instances of classes in >>> an address space, mainly to not have to cope with the handling of the >>> this pointer.  That is, >>> >>>    __flash Myclass *t; >>> >>> does not work.  Nevertheless, I admit that this is would be nice to >>> have. >>> >>>> One big complaint about avr-g++ is that there is no way to put >>>> vtables in >>>> flash (address-space 1) and to access them accordingly.  How can >>>> this be >>>> achieved with C++ address spaces? >>> >>> Do you want only the vtables in the flash address space or do you want >>> to be able to have the whole class content. >> >> My question is about vtables, not the bits that represent some object. >> vtables are stored independently of objects, usually in .rodata + >> comdat.  Notice that vtables are read-only and in static storage, even >> if objects are neither. >> >> The problem with vtables is that the user has no handle to specify >> where to locate them -- and even if, due to AVR quirks, the right >> instruction must be used.  Thus just putting vtables in flash by means >> of some section attribute won't work, only address-spaces can do the >> trick. >> >>> 1. If you only want the vtables, I think that a target hook called >>> at vtable creation would do the trick. >> >> Yes, that would be enough, see https://gcc.gnu.org/PR43745 > > As you say there, this would be an ABI change, so there would need to be > a transition strategy.  I don't know to what extent AVR users try to use > older compiled code vs. always rebuilding everything. > >> Johann >> >>> 2. If you want to be able to have pointer to classes in __flash, I >>> will need to further the support I have currently implemented to >>> support the pointer this qualified with an address space. >>> Retrospectively, I think this have to be implemented. >>> >>> Paul >> >> Would be great if this would work, but I think this can be really >> tricky, because it's already tricky for non-class objects. > > Indeed, especially if objects of the same type can live either in flash > or RAM: you'd need 2 or more of each method for the different accesses. > Perhaps via cloning. > > Simpler might be to declare that objects of a particular class can only > live in flash. > >> A user has to specify __flash explicitly, which is quite different to >> plain objects.  For example, a const int can live in .rodata, but in >> cases like >> >> extern int func(); >> extern const int ival; >> const int ival = func(); >> >> ival would live in .bss and be initialized at runtime by a static >> constructor. Consequently, >> >> const __flash int ival = func(); >> >> is invalid code that has to be diagnosed [1], because in the avr case, >> __flash means non-volatile memory, which contradicts initialization at >> runtime. >> >> So only objects that are TREE_READONLY can go into AS __flash, >> TREE_CONST is not enough. >> >> How is this problem addressed?  Does this require a new target hook to >> diagnose such cases, and does the compiler know at that stage that an >> object will be TREE_READONLY? > > That does sound like a job for a new target hook, if there isn't a > suitable one already. > >> [1] Notice that in C it's enough to check that __flash is always >> accompanied by const, but in C++ this is not more enough as shown in >> the example above. >> >> Johann >> >> >> p.s: I just checked clang v16, it doesn't complain and accepts >> nonsensical code like: >> >> extern int func(); >> >> extern const __flash int cval; >> const __flash int cval = func(); >> >> int get_cval() >> { >>      return cval; >> } >> >> (clang puts cval into .bss and initializes at startup, but get_cval >> will read from flash due to __flash.) >> >