This is an archive of the discontinued LLVM Phabricator instance.

[NFC] Remove redundant loads when has_device_addr is used.
ClosedPublic

Authored by jyu2 on Nov 2 2022, 8:51 PM.

Details

Summary

It is caused by regenerate captured var value when processing the
has_device_addr, the captured var value has been generated in
GenerateOpenMPCapturedVars and passed as Arg in generateInfoForCapture.
The fix just use Arg instead regenerate, just same as is_device_ptr.

Diff Detail

Event Timeline

jyu2 created this revision.Nov 2 2022, 8:51 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 2 2022, 8:51 PM
jyu2 requested review of this revision.Nov 2 2022, 8:51 PM
Herald added a project: Restricted Project. · View Herald Transcript

Whyt it is NFC? I assume it fixes the dereferencing of references, right?

jyu2 added a comment.Nov 3 2022, 2:31 PM

Whyt it is NFC? I assume it fixes the dereferencing of references, right?

Thanks Alexey.

I really don't expect run time change for this change. The change only change BP/PP of the map instead I was using

CGF.EmitLValue(E).getPointer(CGF);

or

CGF.EmitScalarExpr(E);

to generate BP/PP.

Do we have a runtime test for this? Would be good to try to test it if the offloading fails and the host version is executed instead.

jyu2 added a comment.Nov 3 2022, 3:14 PM

Do we have a runtime test for this? Would be good to try to test it if the offloading fails and the host version is executed instead.

I have runtime test in https://reviews.llvm.org/D134268
openmp/libomptarget/test/mapping/target_has_device_addr.c where

void bar() {

short x[10];
short *xp = &x[0];

x[1] = 111;

#pragma omp target data map(tofrom : xp [0:2]) use_device_addr(xp [0:2])
#pragma omp target has_device_addr(xp [0:2])

{
  xp[1] = 222;
  // CHECK: 222
  printf("%d %p\n", xp[1], &xp[1]);
}
// CHECK: 222
printf("%d %p\n", xp[1], &xp[1]);

}

Do we have a runtime test for this? Would be good to try to test it if the offloading fails and the host version is executed instead.

I have runtime test in https://reviews.llvm.org/D134268
openmp/libomptarget/test/mapping/target_has_device_addr.c where

void bar() {

short x[10];
short *xp = &x[0];

x[1] = 111;

#pragma omp target data map(tofrom : xp [0:2]) use_device_addr(xp [0:2])
#pragma omp target has_device_addr(xp [0:2])

{
  xp[1] = 222;
  // CHECK: 222
  printf("%d %p\n", xp[1], &xp[1]);
}
// CHECK: 222
printf("%d %p\n", xp[1], &xp[1]);

}

Need to add a reference

jyu2 updated this revision to Diff 473084.Nov 3 2022, 4:54 PM

Thanks Alexey. New runtime test is added.

This revision is now accepted and ready to land.Nov 4 2022, 6:28 AM

nit: this doesn't look like NFC.

ronlieb added a subscriber: ronlieb.Nov 4 2022, 4:27 PM

[AMD Official Use Only - General]

Please look into AMDGPU bot fail
https://lab.llvm.org/buildbot/#/builders/193/builds/21319

Get Outlook for iOShttps://aka.ms/o0ukef


From: Jennifer Yu via Phabricator <reviews@reviews.llvm.org>
Sent: Friday, November 4, 2022 5:40:19 PM
To: jennifer.yu@intel.com <jennifer.yu@intel.com>; abhinav.gaba@intel.com <abhinav.gaba@intel.com>; a.bataev@hotmail.com <a.bataev@hotmail.com>; michael.p.rice@intel.com <michael.p.rice@intel.com>; jdoerfert@anl.gov <jdoerfert@anl.gov>
Cc: i@tianshilei.me <i@tianshilei.me>; openmp-commits@lists.llvm.org <openmp-commits@lists.llvm.org>; stefomeister@gmail.com <stefomeister@gmail.com>; cfe-commits@lists.llvm.org <cfe-commits@lists.llvm.org>; Pandey, Amit <Amit.Pandey@amd.com>; Islam, Saiyedul <Saiyedul.Islam@amd.com>; jeffrey.sandoval@hpe.com <jeffrey.sandoval@hpe.com>; abidmuslim@gmail.com <abidmuslim@gmail.com>; mpique@icloud.com <mpique@icloud.com>; Lieberman, Ron <Ron.Lieberman@amd.com>; greg63706@gmail.com <greg63706@gmail.com>; kkwli0@gmail.com <kkwli0@gmail.com>; zhang.guansong@gmail.com <zhang.guansong@gmail.com>; ravi.narayanaswamy@intel.com <ravi.narayanaswamy@intel.com>; kevin.sala@bsc.es <kevin.sala@bsc.es>; jdenny.ornl@gmail.com <jdenny.ornl@gmail.com>; balaji-sankar-naga-sai-sandeep.kosuri@hpe.com <balaji-sankar-naga-sai-sandeep.kosuri@hpe.com>; misono.tomohiro@fujitsu.com <misono.tomohiro@fujitsu.com>; sunil.shrestha@hpe.com <sunil.shrestha@hpe.com>; michael.hliao@gmail.com <michael.hliao@gmail.com>; Balasubrmanian, Vignesh <Vignesh.Balasubrmanian@amd.com>; chichun.chen@hpe.com <chichun.chen@hpe.com>; Kumar N, Bhuvanendra <Bhuvanendra.KumarN@amd.com>; lin32@llnl.gov <lin32@llnl.gov>; xw111luoye@gmail.com <xw111luoye@gmail.com>; Tomar, Sourabh Singh <SourabhSingh.Tomar@amd.com>; liao6@llnl.gov <liao6@llnl.gov>; sara.royuela@bsc.es <sara.royuela@bsc.es>; deepak.eachempati@hpe.com <deepak.eachempati@hpe.com>; jatin.bhateja@gmail.com <jatin.bhateja@gmail.com>; 1135831309@qq.com <1135831309@qq.com>; gandhi21299@gmail.com <gandhi21299@gmail.com>; tbaeder@redhat.com <tbaeder@redhat.com>; mlekena@skidmore.edu <mlekena@skidmore.edu>; blitzrakete@gmail.com <blitzrakete@gmail.com>; shenhan@google.com <shenhan@google.com>; david.green@arm.com <david.green@arm.com>; Song, Ruiling <Ruiling.Song@amd.com>
Subject: [PATCH] D137313: [NFC] Remove redundant loads when has_device_addr is used.

Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.

jyu2 closed this revision.
jyu2 added a comment.

de14befa7730093ff3d46c7628fa1084f251e98d https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Freviews.llvm.org%2FrGde14befa7730093ff3d46c7628fa1084f251e98d&amp;data=05%7C01%7Cron.lieberman%40amd.com%7C89e7e812ba444904677f08dabeb58de0%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638031984301236587%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=91XJgV2YZU9jqHYiXbrS4d5t95xt2Bro%2FwEMG6SpUY0%3D&amp;reserved=0

Repository:

rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION

https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Freviews.llvm.org%2FD137313%2Fnew%2F&amp;data=05%7C01%7Cron.lieberman%40amd.com%7C89e7e812ba444904677f08dabeb58de0%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638031984301236587%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=o9eGJWg86IGPzt2G4SlgX6VTxpoWRlAEhb2TBO%2BcGTw%3D&amp;reserved=0

https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Freviews.llvm.org%2FD137313&amp;data=05%7C01%7Cron.lieberman%40amd.com%7C89e7e812ba444904677f08dabeb58de0%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638031984301236587%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=VPD4ootaA99%2FEMcRn34ONBJTzQYWvfTqqh2ErRLrSaA%3D&amp;reserved=0