From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=134.134.136.65; helo=mga03.intel.com; envelope-from=liming.gao@intel.com; receiver=edk2-devel@lists.01.org Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 42B6822571B20 for ; Fri, 16 Mar 2018 08:49:53 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 16 Mar 2018 08:56:18 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.48,316,1517904000"; d="scan'208";a="38045405" Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204]) by fmsmga004.fm.intel.com with ESMTP; 16 Mar 2018 08:56:18 -0700 Received: from fmsmsx124.amr.corp.intel.com (10.18.125.39) by FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS) id 14.3.319.2; Fri, 16 Mar 2018 08:56:17 -0700 Received: from shsmsx101.ccr.corp.intel.com (10.239.4.153) by fmsmsx124.amr.corp.intel.com (10.18.125.39) with Microsoft SMTP Server (TLS) id 14.3.319.2; Fri, 16 Mar 2018 08:56:17 -0700 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.226]) by SHSMSX101.ccr.corp.intel.com ([169.254.1.166]) with mapi id 14.03.0319.002; Fri, 16 Mar 2018 23:56:15 +0800 From: "Gao, Liming" To: Pete Batard , "edk2-devel@lists.01.org" CC: Ard Biesheuvel Thread-Topic: [edk2] [PATCH 0/4] Add ARM64 support for VS2017 Thread-Index: AQHTrIu29dk9gfg5rESiLyKfZ75BZKPQ797A//+w0ACAAgX9YP//pwGAgADVPvA= Date: Fri, 16 Mar 2018 15:56:15 +0000 Message-ID: <4A89E2EF3DFEDB4C8BFDE51014F606A14E1E8137@SHSMSX104.ccr.corp.intel.com> References: <20180223095003.6012-1-pete@akeo.ie> <4A89E2EF3DFEDB4C8BFDE51014F606A14E1E7104@SHSMSX104.ccr.corp.intel.com> <71d6b134-d875-2c24-6687-f3b01d0ff9ea@akeo.ie> <4A89E2EF3DFEDB4C8BFDE51014F606A14E1E7EEB@SHSMSX104.ccr.corp.intel.com> In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH 0/4] Add ARM64 support for VS2017 X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 16 Mar 2018 15:49:53 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Pete: I understand the existing .S file has the inconsistent comment style. I a= lso know new added ASM files are converted from .S files. But, my comment i= s for this patch that adds new ASM files. I expect new added ASM files have= the same style. If you check ARM arch ASM files, you will find they all ha= ve the same style.=20 Thanks Liming > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Pe= te Batard > Sent: Friday, March 16, 2018 7:04 PM > To: edk2-devel@lists.01.org > Cc: Gao, Liming ; Ard Biesheuvel > Subject: Re: [edk2] [PATCH 0/4] Add ARM64 support for VS2017 >=20 > On 2018.03.16 08:24, Gao, Liming wrote: > > Pete: > > .S for GCC assembly, .asm for MSFT assembly. They can have the diff= erent comment style. >=20 > Yes, but as I explained, the actual original issue is that our current > .S files do *not* have the same comment styles in the first place. >=20 > If you look at MdePkg/Library/BaseLib/AArch64/SwitchStack.S, you'll see > that is uses '//' for comments, whereas other .S files, such as > MdePkg/Library/BaseLib/AArch64/SetJumpLongJump.S, use '#'. >=20 > So that is our actual issue here: Regardless of VS2017, the GCC assembly > files for AARCH64 we currently have do not use the same comment style. >=20 > Thus, the only reason why the .asm don't have the same comment style in > our proposal is because the .S, which we derived the .asm from, don't. > This means that either we should fix the .S too, or we shouldn't fix > anything at all. >=20 > > Here, my comment is to make sure .asm files have the same comment st= yle. I don't request to change .S file. >=20 > And what I am saying is that it makes little sense to harmonize the > comment style for the .asm files, if we're not going to do the same for > the .S files as well. It just doesn't seem fair in my book to have the > VS2017 assembly files held to a higher standard than the GCC ones. So > either we need to fix both, or we fix none at all. >=20 > But as I indicated in my last e-mail, I am planning to send an > additional patch that does comment harmonization, for both .S and .asm, > *after* this VS2017 series has been applied to mainline. So the change > you request will happen. Just not as part of this patch series. >=20 > And the reason I have insist on splitting these changes is because, if > we have to alter the .S files to be consistent, then this comment > harmonization request should logically be handled separately from the > VS2017 effort. >=20 > Please let me know if you still think having a future separate patch, > that will do .S and .asm comment harmonization, does not make sense. >=20 > Regards, >=20 > /Pete >=20 > > > >> -----Original Message----- > >> From: Pete Batard [mailto:pete@akeo.ie] > >> Sent: Thursday, March 15, 2018 5:28 PM > >> To: Gao, Liming ; edk2-devel@lists.01.org > >> Cc: ard.biesheuvel@linaro.org > >> Subject: Re: [PATCH 0/4] Add ARM64 support for VS2017 > >> > >> Hi Liming, > >> > >> Thanks for reviewing the patches. > >> > >> On 2018.03.15 06:15, Gao, Liming wrote: > >>> Pete: > >>> For new added ASM file in BaseLib, could you use the same comment= style > >>> for them? ASM use ; for the comment. Most of new files uses ; as the > >>> comment, but switchstack is not. > >> > >> This is because SwitchStack.asm is simply SwitchStack.S, with the GCC > >> assembler specifics removed, and MSVC assembler specifics added. > >> > >> I did not change the comment style from the original files, so the rea= l > >> issue here is that our GCC assembly files for AARCH64 do not use the > >> same comment style. > >> > >> I'm fine with trying to harmonize the comment styles, but seeing as th= is > >> needs to be done for both the .S and .asm, I'd rather send a patch to = do > >> that *after* these VS2017 changes have been applied, as I don't consid= er > >> this correction to in scope of this patch series (because logically, t= he > >> introduction of VS2017 should not alter any of the .S files, unless we > >> reuse them, which we don't). > >> > >> If you agree to apply this series, I'll make sure to send a non > >> VS2017-specific additional patch, that does what you request for both > >> the .S and .asm. > >> > >>> Besides, compared to Arm arch assembly > >>> file, I don't find CpuPause.asm. Is it required? > >> > >> That file doesn't exist for GCC (as you will see there is no > >> MdePkg/Library/BaseLib/AArch64/CpuPause.S), so we don't have one for > >> VS2017 either. > >> > >> Regards, > >> > >> /Pete >=20 > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel