From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: None (no SPF record) identity=mailfrom; client-ip=2a00:1450:400c:c09::22e; helo=mail-wm0-x22e.google.com; envelope-from=pete@akeo.ie; receiver=edk2-devel@lists.01.org Received: from mail-wm0-x22e.google.com (mail-wm0-x22e.google.com [IPv6:2a00:1450:400c:c09::22e]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 7A9CC22489CBE for ; Fri, 16 Mar 2018 03:57:18 -0700 (PDT) Received: by mail-wm0-x22e.google.com with SMTP id t6so2269743wmt.5 for ; Fri, 16 Mar 2018 04:03:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=akeo-ie.20150623.gappssmtp.com; s=20150623; h=subject:to:references:from:cc:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=9476p5MeDPG3X9j5DZ3g51mNG5YD4QDpWJ3iSgVH4EU=; b=JQVB7zK5OWfmIr8KfURDDb+IRtXqFqaxJWBWcNJNGW99YKytp9I1gM+IxtDolM6J92 sBQI1DFGXND7iZMy9bmO3klcfjZeHr1jGK4hPk5wOd+smHnO7uky4M6/W1jAt18SbzYn 5i140eTv5qgLsLViqBBcF/6xis5sZoP4X3+hQIimtdjEvChfl23qyNDq+tqBjkejaEL0 nax2iKcolRfYZAv7kNQ6WeXs8y2uia8zwfNwJ7lq868fr7Nw0BmfAWURuoIMtRtO3C8E N9xLdtG2RCX5NLHtXiww+MP9HCdHFn0ydJ+BIB0t3hEREYIXqQc/TD8rXMND4T7r4cwe nFhA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:cc:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=9476p5MeDPG3X9j5DZ3g51mNG5YD4QDpWJ3iSgVH4EU=; b=nCVS57pwQYFsWsKe/PnXhh7H4IP5ZCPoy5piDrOZjNy1rqi0gPcLAjnsgsQHRNAvd+ G7u0pvD8+UOCEVxAJ/yrpqU4lONfId2ImMhbNk/IC3gYcF/5F8BZ+f89LvsdyK0VQZSM g1lQ3ubRZ2bidXye5sz3+pa6/ITzqMjoBdFJNIiWF8YW4N5orGvEYJm1B04fCPLWjD8m 9PJUKnLQjcQbGBe7S2UxYcxkOhz4+wKiheRkUkWRT/RU5xNqy07wS1kCeI+93kDcnIIn 0mjxNVTq7L5Y1s18Xktkta3qFsOkoramWEO7tZrjp+5yeuKFUyY01cBcls5OHB+El5bq jcjQ== X-Gm-Message-State: AElRT7FGaxpEiLG7RBNRoohWeNtUN2/fuoHIQI8H0ZX0JGU85aV3Y3EY mJaFkHg9qKIhlTYNXhsafzL0FA== X-Google-Smtp-Source: AG47ELumZjJiUp9tFzksAU5rqaHSxNmlEYgFAOnJhL5HNfkdbc8k9FGoC9P/NDgHPfsJ1CmKR+ayzA== X-Received: by 10.80.151.214 with SMTP id f22mr1984206edb.163.1521198221852; Fri, 16 Mar 2018 04:03:41 -0700 (PDT) Received: from [10.0.0.101] ([84.203.73.68]) by smtp.googlemail.com with ESMTPSA id e56sm3813579edb.56.2018.03.16.04.03.40 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 16 Mar 2018 04:03:40 -0700 (PDT) To: "edk2-devel@lists.01.org" 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> From: Pete Batard Cc: "Gao, Liming" , Ard Biesheuvel Message-ID: Date: Fri, 16 Mar 2018 11:03:39 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 MIME-Version: 1.0 In-Reply-To: <4A89E2EF3DFEDB4C8BFDE51014F606A14E1E7EEB@SHSMSX104.ccr.corp.intel.com> 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 10:57:20 -0000 Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-GB Content-Transfer-Encoding: 7bit On 2018.03.16 08:24, Gao, Liming wrote: > Pete: > .S for GCC assembly, .asm for MSFT assembly. They can have the different comment style. 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. 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 '#'. 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. 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. > Here, my comment is to make sure .asm files have the same comment style. I don't request to change .S file. 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. 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. 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. 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. Regards, /Pete > >> -----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 real >> 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 this >> 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 consider >> this correction to in scope of this patch series (because logically, the >> 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